From ef9d4f4e0615bcc42528e2e73ade0ba02baa3ed9 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 8 Oct 2016 00:01:22 +0200 Subject: [PATCH] Use reselect to memoize denormalization in UI state Also upgrade react-redux to latest version. This is a performance update --- app/assets/javascripts/components.js | 1 + .../components/components/status_list.jsx | 2 +- .../components/features/account/index.jsx | 14 ++-- .../features/public_timeline/index.jsx | 40 ++++----- .../components/features/status/index.jsx | 16 ++-- .../ui/containers/compose_form_container.jsx | 4 +- .../ui/containers/notifications_container.jsx | 8 +- .../ui/containers/status_list_container.jsx | 16 ++-- .../components/reducers/timelines.jsx | 26 ------ .../components/selectors/index.jsx | 81 +++++++++++++++++++ app/assets/stylesheets/components.scss | 2 +- package.json | 6 +- 12 files changed, 136 insertions(+), 80 deletions(-) create mode 100644 app/assets/javascripts/components/selectors/index.jsx diff --git a/app/assets/javascripts/components.js b/app/assets/javascripts/components.js index 16e33e58e..0ac74c70c 100644 --- a/app/assets/javascripts/components.js +++ b/app/assets/javascripts/components.js @@ -3,6 +3,7 @@ window.React = require('react'); window.ReactDOM = require('react-dom'); +window.Perf = require('react-addons-perf'); //= require_tree ./components diff --git a/app/assets/javascripts/components/components/status_list.jsx b/app/assets/javascripts/components/components/status_list.jsx index 9855ec141..f70d53263 100644 --- a/app/assets/javascripts/components/components/status_list.jsx +++ b/app/assets/javascripts/components/components/status_list.jsx @@ -28,7 +28,7 @@ const StatusList = React.createClass({ const { statuses, onScrollToBottom, ...other } = this.props; return ( -
+
{statuses.map((status) => { return ; diff --git a/app/assets/javascripts/components/features/account/index.jsx b/app/assets/javascripts/components/features/account/index.jsx index 9c77214d1..21aa8c5d6 100644 --- a/app/assets/javascripts/components/features/account/index.jsx +++ b/app/assets/javascripts/components/features/account/index.jsx @@ -20,22 +20,18 @@ import { } from '../../actions/interactions'; import Header from './components/header'; import { - selectStatus, - selectAccount -} from '../../reducers/timelines'; + getAccountTimeline, + getAccount +} from '../../selectors'; import StatusList from '../../components/status_list'; import LoadingIndicator from '../../components/loading_indicator'; import Immutable from 'immutable'; import ActionBar from './components/action_bar'; import Column from '../ui/components/column'; -function selectStatuses(state, accountId) { - return state.getIn(['timelines', 'accounts_timelines', accountId], Immutable.List([])).map(id => selectStatus(state, id)).filterNot(status => status === null); -}; - const mapStateToProps = (state, props) => ({ - account: selectAccount(state, Number(props.params.accountId)), - statuses: selectStatuses(state, Number(props.params.accountId)), + account: getAccount(state, Number(props.params.accountId)), + statuses: getAccountTimeline(state, Number(props.params.accountId)), me: state.getIn(['timelines', 'me']) }); diff --git a/app/assets/javascripts/components/features/public_timeline/index.jsx b/app/assets/javascripts/components/features/public_timeline/index.jsx index dd31dc115..450725af6 100644 --- a/app/assets/javascripts/components/features/public_timeline/index.jsx +++ b/app/assets/javascripts/components/features/public_timeline/index.jsx @@ -1,33 +1,35 @@ -import { connect } from 'react-redux'; -import PureRenderMixin from 'react-addons-pure-render-mixin'; -import ImmutablePropTypes from 'react-immutable-proptypes'; -import StatusList from '../../components/status_list'; -import Column from '../ui/components/column'; -import Immutable from 'immutable'; -import { selectStatus } from '../../reducers/timelines'; +import { connect } from 'react-redux'; +import PureRenderMixin from 'react-addons-pure-render-mixin'; +import ImmutablePropTypes from 'react-immutable-proptypes'; +import StatusList from '../../components/status_list'; +import Column from '../ui/components/column'; +import Immutable from 'immutable'; +import { makeGetTimeline } from '../../selectors'; import { updateTimeline, refreshTimeline, expandTimeline -} from '../../actions/timelines'; -import { deleteStatus } from '../../actions/statuses'; -import { replyCompose } from '../../actions/compose'; +} from '../../actions/timelines'; +import { deleteStatus } from '../../actions/statuses'; +import { replyCompose } from '../../actions/compose'; import { favourite, reblog, unreblog, unfavourite -} from '../../actions/interactions'; +} from '../../actions/interactions'; -function selectStatuses(state) { - return state.getIn(['timelines', 'public'], Immutable.List()).map(id => selectStatus(state, id)).filterNot(status => status === null); +const makeMapStateToProps = () => { + const getTimeline = makeGetTimeline(); + + const mapStateToProps = (state) => ({ + statuses: getTimeline(state, 'public'), + me: state.getIn(['timelines', 'me']) + }); + + return mapStateToProps; }; -const mapStateToProps = (state) => ({ - statuses: selectStatuses(state), - me: state.getIn(['timelines', 'me']) -}); - const PublicTimeline = React.createClass({ propTypes: { @@ -100,4 +102,4 @@ const PublicTimeline = React.createClass({ }); -export default connect(mapStateToProps)(PublicTimeline); +export default connect(makeMapStateToProps)(PublicTimeline); diff --git a/app/assets/javascripts/components/features/status/index.jsx b/app/assets/javascripts/components/features/status/index.jsx index b282956b1..1d40f127b 100644 --- a/app/assets/javascripts/components/features/status/index.jsx +++ b/app/assets/javascripts/components/features/status/index.jsx @@ -10,16 +10,16 @@ import ActionBar from './components/action_bar'; import Column from '../ui/components/column'; import { favourite, reblog } from '../../actions/interactions'; import { replyCompose } from '../../actions/compose'; -import { selectStatus } from '../../reducers/timelines'; - -function selectStatuses(state, ids) { - return ids.map(id => selectStatus(state, id)).filterNot(status => status === null); -}; +import { + getStatus, + getStatusAncestors, + getStatusDescendants +} from '../../selectors'; const mapStateToProps = (state, props) => ({ - status: selectStatus(state, Number(props.params.statusId)), - ancestors: selectStatuses(state, state.getIn(['timelines', 'ancestors', Number(props.params.statusId)], Immutable.OrderedSet())), - descendants: selectStatuses(state, state.getIn(['timelines', 'descendants', Number(props.params.statusId)], Immutable.OrderedSet())), + status: getStatus(state, Number(props.params.statusId)), + ancestors: getStatusAncestors(state, Number(props.params.statusId)), + descendants: getStatusDescendants(state, Number(props.params.statusId)), me: state.getIn(['timelines', 'me']) }); diff --git a/app/assets/javascripts/components/features/ui/containers/compose_form_container.jsx b/app/assets/javascripts/components/features/ui/containers/compose_form_container.jsx index 2427f89f0..747eb9691 100644 --- a/app/assets/javascripts/components/features/ui/containers/compose_form_container.jsx +++ b/app/assets/javascripts/components/features/ui/containers/compose_form_container.jsx @@ -1,14 +1,14 @@ import { connect } from 'react-redux'; import ComposeForm from '../components/compose_form'; import { changeCompose, submitCompose, cancelReplyCompose } from '../../../actions/compose'; -import { selectStatus } from '../../../reducers/timelines'; +import { getStatus } from '../../../selectors'; const mapStateToProps = function (state, props) { return { text: state.getIn(['compose', 'text']), is_submitting: state.getIn(['compose', 'is_submitting']), is_uploading: state.getIn(['compose', 'is_uploading']), - in_reply_to: selectStatus(state, state.getIn(['compose', 'in_reply_to'])) + in_reply_to: getStatus(state, state.getIn(['compose', 'in_reply_to'])) }; }; diff --git a/app/assets/javascripts/components/features/ui/containers/notifications_container.jsx b/app/assets/javascripts/components/features/ui/containers/notifications_container.jsx index bc339ef28..eb12989e5 100644 --- a/app/assets/javascripts/components/features/ui/containers/notifications_container.jsx +++ b/app/assets/javascripts/components/features/ui/containers/notifications_container.jsx @@ -4,14 +4,10 @@ import { dismissNotification, clearNotifications } from '../../../actions/notifications'; +import { getNotifications } from '../../../selectors'; const mapStateToProps = (state, props) => ({ - notifications: state.get('notifications').map((item, i) => ({ - message: item.get('message'), - title: item.get('title'), - key: item.get('key'), - dismissAfter: 5000 - })).toJS() + notifications: getNotifications(state) }); const mapDispatchToProps = (dispatch) => { diff --git a/app/assets/javascripts/components/features/ui/containers/status_list_container.jsx b/app/assets/javascripts/components/features/ui/containers/status_list_container.jsx index 605216eba..045cc59d1 100644 --- a/app/assets/javascripts/components/features/ui/containers/status_list_container.jsx +++ b/app/assets/javascripts/components/features/ui/containers/status_list_container.jsx @@ -8,14 +8,18 @@ import { unfavourite } from '../../../actions/interactions'; import { expandTimeline } from '../../../actions/timelines'; -import { selectStatus } from '../../../reducers/timelines'; +import { makeGetTimeline } from '../../../selectors'; import { deleteStatus } from '../../../actions/statuses'; -const mapStateToProps = function (state, props) { - return { - statuses: state.getIn(['timelines', props.type]).map(id => selectStatus(state, id)), +const makeMapStateToProps = () => { + const getTimeline = makeGetTimeline(); + + const mapStateToProps = (state, props) => ({ + statuses: getTimeline(state, props.type), me: state.getIn(['timelines', 'me']) - }; + }); + + return mapStateToProps; }; const mapDispatchToProps = function (dispatch, props) { @@ -50,4 +54,4 @@ const mapDispatchToProps = function (dispatch, props) { }; }; -export default connect(mapStateToProps, mapDispatchToProps)(StatusList); +export default connect(makeMapStateToProps, mapDispatchToProps)(StatusList); diff --git a/app/assets/javascripts/components/reducers/timelines.jsx b/app/assets/javascripts/components/reducers/timelines.jsx index 0b02ac181..927ac28fd 100644 --- a/app/assets/javascripts/components/reducers/timelines.jsx +++ b/app/assets/javascripts/components/reducers/timelines.jsx @@ -40,32 +40,6 @@ const initialState = Immutable.Map({ relationships: Immutable.Map() }); -export function selectStatus(state, id) { - let status = state.getIn(['timelines', 'statuses', id], null); - - if (status === null) { - return null; - } - - status = status.set('account', selectAccount(state, status.get('account'))); - - if (status.get('reblog') !== null) { - status = status.set('reblog', selectStatus(state, status.get('reblog'))); - } - - return status; -}; - -export function selectAccount(state, id) { - let account = state.getIn(['timelines', 'accounts', id], null); - - if (account === null) { - return null; - } - - return account.set('relationship', state.getIn(['timelines', 'relationships', id])); -}; - function normalizeStatus(state, status) { // Separate account let account = status.get('account'); diff --git a/app/assets/javascripts/components/selectors/index.jsx b/app/assets/javascripts/components/selectors/index.jsx new file mode 100644 index 000000000..a40cb15da --- /dev/null +++ b/app/assets/javascripts/components/selectors/index.jsx @@ -0,0 +1,81 @@ +import { createSelector } from 'reselect' +import Immutable from 'immutable'; + +const getStatuses = state => state.getIn(['timelines', 'statuses']); +const getAccounts = state => state.getIn(['timelines', 'accounts']); + +const getAccountBase = (state, id) => state.getIn(['timelines', 'accounts', id], null); +const getAccountRelationship = (state, id) => state.getIn(['timelines', 'relationships', id]); + +export const getAccount = createSelector([getAccountBase, getAccountRelationship], (base, relationship) => { + if (base === null) { + return null; + } + + return base.set('relationship', relationship); +}); + +const getStatusBase = (state, id) => state.getIn(['timelines', 'statuses', id], null); + +export const getStatus = createSelector([getStatusBase, getStatuses, getAccounts], (base, statuses, accounts) => { + if (base === null) { + return null; + } + + return assembleStatus(base.get('id'), statuses, accounts); +}); + +const getAccountTimelineIds = (state, id) => state.getIn(['timelines', 'accounts_timelines', id], Immutable.List()); + +const assembleStatus = (id, statuses, accounts) => { + let status = statuses.get(id); + + if (status === null) { + return null; + } + + let reblog = statuses.get(status.get('reblog'), null); + + if (reblog !== null) { + reblog = reblog.set('account', accounts.get(reblog.get('account'))); + } + + return status.set('reblog', reblog).set('account', accounts.get(status.get('account'))); +}; + +const assembleStatusList = (ids, statuses, accounts) => { + return ids.map(statusId => assembleStatus(statusId, statuses, accounts)).filterNot(status => status === null); +}; + +export const getAccountTimeline = createSelector([getAccountTimelineIds, getStatuses, getAccounts], assembleStatusList); + +const getTimelineIds = (state, timelineType) => state.getIn(['timelines', timelineType]); + +export const makeGetTimeline = () => { + return createSelector([getTimelineIds, getStatuses, getAccounts], assembleStatusList); +}; + +const getStatusAncestorsIds = (state, id) => state.getIn(['timelines', 'ancestors', id], Immutable.OrderedSet()); + +export const getStatusAncestors = createSelector([getStatusAncestorsIds, getStatuses, getAccounts], assembleStatusList); + +const getStatusDescendantsIds = (state, id) => state.getIn(['timelines', 'descendants', id], Immutable.OrderedSet()); + +export const getStatusDescendants = createSelector([getStatusDescendantsIds, getStatuses, getAccounts], assembleStatusList); + +const getNotificationsBase = state => state.get('notifications'); + +export const getNotifications = createSelector([getNotificationsBase], (base) => { + let arr = []; + + base.forEach(item => { + arr.push({ + message: item.get('message'), + title: item.get('title'), + key: item.get('key'), + dismissAfter: 5000 + }); + }); + + return arr; +}); diff --git a/app/assets/stylesheets/components.scss b/app/assets/stylesheets/components.scss index 348857393..78419988d 100644 --- a/app/assets/stylesheets/components.scss +++ b/app/assets/stylesheets/components.scss @@ -198,7 +198,7 @@ font-size: 13px; display: block; padding: 6px 16px; - width: 120px; + width: 100px; text-decoration: none; background: #d9e1e8; color: #282c37; diff --git a/package.json b/package.json index 008f66ded..090f99550 100644 --- a/package.json +++ b/package.json @@ -17,15 +17,17 @@ "es6-promise": "^3.2.1", "immutable": "^3.8.1", "moment": "^2.14.1", + "react-addons-perf": "^15.3.2", "react-addons-pure-render-mixin": "^15.3.1", "react-immutable-proptypes": "^2.1.0", "react-notification": "^6.1.1", - "react-redux": "^4.4.5", + "react-redux": "^5.0.0-beta.3", "react-redux-loading-bar": "^2.3.3", "react-router": "^2.8.0", "react-simple-dropdown": "^1.1.4", "redux": "^3.5.2", "redux-immutable": "^3.0.8", - "redux-thunk": "^2.1.0" + "redux-thunk": "^2.1.0", + "reselect": "^2.5.4" } }