Skip to content

perf: Cache posts in redux store or performance #86

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chauduong1192 wants to merge 3 commits into
base: master
Choose a base branch
from
Open

perf: Cache posts in redux store or performance #86

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c246a1d
perf: Cache posts in redux store or performance
chauduong1192 Jan 14, 2018
2556be4
perf: Change code to improve app
chauduong1192 Jan 14, 2018
2b745d9
fix: In progress
chauduong1192 Feb 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/App.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const routes = [{
render: HomeRender,
}, {
key: 'source',
path: '/source/:source',
path: '/source/:sourceId',
render: HomeRender,
}, {
key: 'for-you',
Expand Down
7 changes: 1 addition & 6 deletions src/components/Common/Sort/Sort.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,15 @@ import './Sort.scss';

const propTypes = {
current: PropTypes.string.isRequired,
onClick: PropTypes.func.isRequired,
};

const Sort = ({ current, onClick }) => {
const Sort = ({ current }) => {
const items = [{
display: 'New',
value: 'new',
}, {
display: 'Best',
value: 'best',
// }, {
// display: 'Daily',
// value: 'daily',
}];

return (
Expand All @@ -30,7 +26,6 @@ const Sort = ({ current, onClick }) => {
to={{
search: `?sort=${value}`,
}}
onClick={() => onClick(value)}
>{display}</Link>
</li>
))}
Expand Down
73 changes: 32 additions & 41 deletions src/components/ForYou/ForYou.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import React, { Component } from 'react';
import { Users } from '@gorillab/reader-js';
import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import qs from 'qs';

import { getForYouPosts, postsSelectors } from '../../state/ducks/posts';

import PageHeader from '../Common/PageHeader';
import PageTitle from '../Common/PageTitle';
import Sort from '../Common/Sort';
Expand All @@ -12,6 +15,11 @@ import './ForYou.scss';

const LIMIT = 25;

const propTypes = {
getPosts: PropTypes.func.isRequired,
getForYouPosts: PropTypes.func.isRequired,
};

class ForYou extends Component {
constructor(props) {
super(props);
Expand All @@ -26,52 +34,28 @@ class ForYou extends Component {
};

this.getMore = this.getMore.bind(this);
this.changeSort = this.changeSort.bind(this);
}

componentDidMount() {
this.getPosts();
}

async getPosts(query = {}) {
try {
const posts = await Users.getForYou({
async componentDidMount() {
if (!this.props.getPosts(this.state.sort).length) {
await this.props.getForYouPosts({
sort: this.state.sort,
limit: this.state.limit,
page: this.state.page,
...query,
});

this.setState({
posts,
...query,
});
} catch (error) {
// eslint-disable-next-line no-console
console.error(error);
}
// eslint-disable-next-line react/no-did-mount-set-state
this.setState({
posts: [...this.props.getPosts(this.state.sort)],
});
}

async getMore() {
try {
const posts = await Users.getForYou({
sort: this.state.sort,
limit: LIMIT,
page: Math.floor(this.state.posts.length / LIMIT) + 1,
});

this.setState({
posts: this.state.posts.concat(posts),
limit: this.state.limit + LIMIT,
});
} catch (error) {
// eslint-disable-next-line no-console
console.error(error);
}
}

changeSort(sort) {
this.getPosts({ sort });
getMore() {
this.props.getForYouPosts({
sort: this.state.sort,
limit: LIMIT,
page: Math.floor(this.props.getPosts(this.state.sort).length / LIMIT) + 1,
});
}

render() {
Expand All @@ -80,7 +64,7 @@ class ForYou extends Component {
<PageHeader>
<PageTitle title="For You" />

<Sort current={this.state.sort} onClick={this.changeSort} />
<Sort current={this.state.sort} />
</PageHeader>

<PageContent>
Expand All @@ -95,5 +79,12 @@ class ForYou extends Component {
);
}
}

export default ForYou;
ForYou.propTypes = propTypes;

export default connect(
state => ({
getPosts: postsSelectors.getForYouPosts(state),
}), {
getForYouPosts,
},
)(ForYou);
92 changes: 49 additions & 43 deletions src/components/Home/Home.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import React, { Component } from 'react';
import PropTypes from 'prop-types';
import { Posts } from '@gorillab/reader-js';
import { connect } from 'react-redux';
import qs from 'qs';

import { getHomePosts, getSourcePosts, postsSelectors } from '../../state/ducks/posts';

import PageHeader from '../Common/PageHeader';
import PageTitle from '../Common/PageTitle';
import Sort from '../Common/Sort';
Expand All @@ -18,6 +20,12 @@ const propTypes = {
isLoggedIn: PropTypes.bool.isRequired,
title: PropTypes.string.isRequired,
source: PropTypes.any,
getPosts: PropTypes.func.isRequired,
getHomePosts: PropTypes.func.isRequired,
// eslint-disable-next-line react/no-unused-prop-types
getPostsBySource: PropTypes.func.isRequired,
// eslint-disable-next-line react/no-unused-prop-types
match: PropTypes.any.isRequired,
};
const defaultProps = {
source: undefined,
Expand All @@ -28,7 +36,6 @@ class Home extends Component {
super(props);

const { sort } = qs.parse(location.search, { ignoreQueryPrefix: true });

this.state = {
posts: [],
sort: sort || 'new',
Expand All @@ -37,54 +44,45 @@ class Home extends Component {
};

this.getMore = this.getMore.bind(this);
this.changeSort = this.changeSort.bind(this);
}

componentDidMount() {
this.getPosts();
}

async getPosts(query = {}) {
try {
const posts = await Posts.getPosts({
source: this.props.source ? this.props.source.id : undefined,
sort: this.state.sort,
limit: this.state.limit,
page: this.state.page,
...query,
});

async componentDidMount() {
const sourceId = this.props.match.params.sourceId;
if (sourceId) {
if (!this.props.getPostsBySource(this.state.sort, sourceId).length) {
await this.props.getSourcePosts({
source: sourceId,
sort: this.state.sort,
limit: this.state.limit,
page: this.state.page,
});
}
// eslint-disable-next-line react/no-did-mount-set-state
this.setState({
posts,
...query,
posts: [...this.props.getPostsBySource(this.state.sort, sourceId)],
});
} catch (error) {
// eslint-disable-next-line no-console
console.error(error);
}
}

async getMore() {
try {
const posts = await Posts.getPosts({
source: this.props.source ? this.props.source.id : undefined,
sort: this.state.sort,
limit: LIMIT,
page: Math.floor(this.state.posts.length / LIMIT) + 1,
});

} else {
if (!this.props.getPosts(this.state.sort).length) {
await this.props.getHomePosts({
sort: this.state.sort,
limit: this.state.limit,
page: this.state.page,
});
}
// eslint-disable-next-line react/no-did-mount-set-state
this.setState({
posts: this.state.posts.concat(posts),
limit: this.state.limit + LIMIT,
posts: [...this.props.getPosts(this.state.sort)],
});
} catch (error) {
// eslint-disable-next-line no-console
console.error(error);
}
}

changeSort(sort) {
this.getPosts({ sort });
getMore() {
this.props.getHomePosts({
source: this.props.source ? this.props.source.id : undefined,
sort: this.state.sort,
limit: LIMIT,
page: Math.floor(this.props.getPosts(this.state.sort).length / LIMIT) + 1,
});
}

render() {
Expand All @@ -96,7 +94,7 @@ class Home extends Component {
{this.props.isLoggedIn && this.props.source
&& <SubscribeButton source={this.props.source} />}

<Sort current={this.state.sort} onClick={this.changeSort} />
<Sort current={this.state.sort} />
</PageHeader>

<PageContent>
Expand All @@ -115,4 +113,12 @@ class Home extends Component {
Home.propTypes = propTypes;
Home.defaultProps = defaultProps;

export default Home;
export default connect(
state => ({
getPosts: postsSelectors.getHomePosts(state),
getPostsBySource: postsSelectors.getSourcePosts(state),
}), {
getHomePosts,
getSourcePosts,
},
)(Home);
2 changes: 1 addition & 1 deletion src/components/Home/Home.render.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const HomeRender = ({ match, getSource, ...rest }) => {
const source = match.params.source ? getSource(match.params.source) : undefined;
const title = source ? source.title : 'Explore';

return <Home title={title} source={source} {...rest} />;
return <Home title={title} source={source} {...rest} match={match} />;
Copy link
Copy Markdown
Contributor

@phatpham9 phatpham9 Jan 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need to pass match to Home props anymore because I extracted the source id (source) from it already in

const source = match.params.source ? getSource(match.params.source) : undefined;

return <Home title={title} source={source} {...rest} />;

Copy link
Copy Markdown
Contributor

@phatpham9 phatpham9 Jan 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe you just need to change source to sourceId

const sourceId = match.params.sourceId ? getSource(match.params.sourceId) : undefined;

return <Home title={title} sourceId={sourceId} {...rest} />;

};

HomeRender.propTypes = propTypes;
Expand Down
Loading