Reduce Unnecessary Re-renders in Revenue Report (https://github.com/woocommerce/woocommerce-admin/pull/5986)

* Avoid new references in getReportChartData() return.
* Avoid some new references in ReportTable component.
* Avoid some new references, fix cache key for getReportChartData memoization.
* Memoize date package functions used in ReportTable.
* Avoid more new references in RevenueReportTable.
This commit is contained in:
Jeff Stieler 2021-01-06 18:01:45 -07:00 committed by GitHub
parent e908bb19c2
commit 0c951a4652
5 changed files with 315 additions and 126 deletions

View File

@ -563,6 +563,9 @@ ReportTable.defaultProps = {
baseSearchQuery: {},
};
const EMPTY_ARRAY = [];
const EMPTY_OBJECT = {};
export default compose(
withSelect( ( select, props ) => {
const {
@ -583,7 +586,7 @@ export default compose(
( query.search &&
! ( query[ endpoint ] && query[ endpoint ].length ) )
) {
return {};
return EMPTY_OBJECT;
}
const { woocommerce_default_date_range: defaultDateRange } = select(
SETTINGS_STORE_NAME
@ -599,11 +602,10 @@ export default compose(
select,
filters,
advancedFilters,
tableQuery,
defaultDateRange,
fields: summaryFields,
} )
: {};
: EMPTY_OBJECT;
const queriedTableData =
tableData ||
getReportTableData( {
@ -628,9 +630,9 @@ export default compose(
? extendedTableData.items.data.map(
( item ) => item[ itemIdField ]
)
: [],
: EMPTY_ARRAY,
tableData: extendedTableData,
query: { ...tableQuery, ...query },
query,
};
} ),
withDispatch( ( dispatch ) => {

View File

@ -6,7 +6,7 @@ import { Component } from '@wordpress/element';
import { format as formatDate } from '@wordpress/date';
import { withSelect } from '@wordpress/data';
import { compose } from '@wordpress/compose';
import { get } from 'lodash';
import { get, memoize } from 'lodash';
import { Date, Link } from '@woocommerce/components';
import { formatValue } from '@woocommerce/number';
import { getSetting } from '@woocommerce/wc-admin-settings';
@ -21,6 +21,7 @@ import {
defaultTableDateFormat,
getCurrentDates,
} from '@woocommerce/date';
import { stringify } from 'qs';
/**
* Internal dependencies
@ -28,6 +29,19 @@ import {
import ReportTable from '../../components/report-table';
import { CurrencyContext } from '../../../lib/currency-context';
const EMPTY_ARRAY = [];
const summaryFields = [
'orders_count',
'gross_sales',
'total_sales',
'refunds',
'coupons',
'taxes',
'shipping',
'net_revenue',
];
class RevenueReportTable extends Component {
constructor() {
super();
@ -257,16 +271,7 @@ class RevenueReportTable extends Component {
getHeadersContent={ this.getHeadersContent }
getRowsContent={ this.getRowsContent }
getSummary={ this.getSummary }
summaryFields={ [
'orders_count',
'gross_sales',
'total_sales',
'refunds',
'coupons',
'taxes',
'shipping',
'net_revenue',
] }
summaryFields={ summaryFields }
query={ query }
tableData={ tableData }
title={ __( 'Revenue', 'woocommerce-admin' ) }
@ -280,6 +285,69 @@ class RevenueReportTable extends Component {
RevenueReportTable.contextType = CurrencyContext;
/**
* Memoized props object formatting function.
*
* @param {boolean} isError
* @param {boolean} isRequesting
* @param {Object} tableQuery
* @param {Object} revenueData
* @return {Object} formatted tableData prop
*/
const formatProps = memoize(
( isError, isRequesting, tableQuery, revenueData ) => ( {
tableData: {
items: {
data: get( revenueData, [ 'data', 'intervals' ], EMPTY_ARRAY ),
totalResults: get( revenueData, [ 'totalResults' ], 0 ),
},
isError,
isRequesting,
query: tableQuery,
},
} ),
( isError, isRequesting, tableQuery, revenueData ) =>
[
isError,
isRequesting,
stringify( tableQuery ),
get( revenueData, [ 'totalResults' ], 0 ),
get( revenueData, [ 'data', 'intervals' ], EMPTY_ARRAY ).length,
].join( ':' )
);
/**
* Memoized table query formatting function.
*
* @param {string} order
* @param {string} orderBy
* @param {number} page
* @param {number} pageSize
* @param {Object} datesFromQuery
* @return {Object} formatted tableQuery object
*/
const formatTableQuery = memoize(
// @todo Support hour here when viewing a single day
( order, orderBy, page, pageSize, datesFromQuery ) => ( {
interval: 'day',
orderby: orderBy,
order,
page,
per_page: pageSize,
after: appendTimestamp( datesFromQuery.primary.after, 'start' ),
before: appendTimestamp( datesFromQuery.primary.before, 'end' ),
} ),
( order, orderBy, page, pageSize, datesFromQuery ) =>
[
order,
orderBy,
page,
pageSize,
datesFromQuery.primary.after,
datesFromQuery.primary.before,
].join( ':' )
);
export default compose(
withSelect( ( select, props ) => {
const { query, filters, advancedFilters } = props;
@ -291,16 +359,13 @@ export default compose(
REPORTS_STORE_NAME
);
// @todo Support hour here when viewing a single day
const tableQuery = {
interval: 'day',
orderby: query.orderby || 'date',
order: query.order || 'desc',
page: query.paged || 1,
per_page: query.per_page || QUERY_DEFAULTS.pageSize,
after: appendTimestamp( datesFromQuery.primary.after, 'start' ),
before: appendTimestamp( datesFromQuery.primary.before, 'end' ),
};
const tableQuery = formatTableQuery(
query.order || 'desc',
query.orderby || 'date',
query.paged || 1,
query.per_page || QUERY_DEFAULTS.pageSize,
datesFromQuery
);
const filteredTableQuery = getReportTableQuery( {
endpoint: 'revenue',
query,
@ -318,16 +383,6 @@ export default compose(
filteredTableQuery,
] );
return {
tableData: {
items: {
data: get( revenueData, [ 'data', 'intervals' ], [] ),
totalResults: get( revenueData, [ 'totalResults' ], 0 ),
},
isError,
isRequesting,
query: tableQuery,
},
};
return formatProps( isError, isRequesting, tableQuery, revenueData );
} )
)( RevenueReportTable );

View File

@ -3,6 +3,8 @@
*/
import { getResourceName } from '../utils';
const EMPTY_OBJECT = {};
export const getReportItemsError = ( state, endpoint, query ) => {
const resourceName = getResourceName( endpoint, query );
return state.itemErrors[ resourceName ] || false;
@ -10,12 +12,12 @@ export const getReportItemsError = ( state, endpoint, query ) => {
export const getReportItems = ( state, endpoint, query ) => {
const resourceName = getResourceName( endpoint, query );
return state.items[ resourceName ] || {};
return state.items[ resourceName ] || EMPTY_OBJECT;
};
export const getReportStats = ( state, endpoint, query ) => {
const resourceName = getResourceName( endpoint, query );
return state.stats[ resourceName ] || {};
return state.stats[ resourceName ] || EMPTY_OBJECT;
};
export const getReportStatsError = ( state, endpoint, query ) => {

View File

@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { find, forEach, isNull, get, includes } from 'lodash';
import { find, forEach, isNull, get, includes, memoize } from 'lodash';
import moment from 'moment';
import {
appendTimestamp,
@ -20,6 +20,7 @@ import {
import * as reportsUtils from './utils';
import { MAX_PER_PAGE, QUERY_DEFAULTS } from '../constants';
import { STORE_NAME } from './constants';
import { getResourceName } from '../utils';
/**
* Add filters and advanced filters values to a query object.
@ -283,6 +284,57 @@ export function getSummaryNumbers( options ) {
};
}
/**
* Static responses object to avoid returning new references each call.
*/
const reportChartDataResponses = {
requesting: {
isEmpty: false,
isError: false,
isRequesting: true,
data: {
totals: {},
intervals: [],
},
},
error: {
isEmpty: false,
isError: true,
isRequesting: false,
data: {
totals: {},
intervals: [],
},
},
empty: {
isEmpty: true,
isError: false,
isRequesting: false,
data: {
totals: {},
intervals: [],
},
},
};
const EMPTY_ARRAY = [];
/**
* Cache helper for returning the full chart dataset after multiple
* requests. Memoized on the request query (string), only called after
* all the requests have resolved successfully.
*/
const getReportChartDataResponse = memoize(
( requestString, totals, intervals ) => ( {
isEmpty: false,
isError: false,
isRequesting: false,
data: { totals, intervals },
} ),
( requestString, totals, intervals ) =>
[ requestString, totals.length, intervals.length ].join( ':' )
);
/**
* Returns all of the data needed to render a chart with summary numbers on a report page.
*
@ -301,16 +353,6 @@ export function getReportChartData( options ) {
STORE_NAME
);
const response = {
isEmpty: false,
isError: false,
isRequesting: false,
data: {
totals: {},
intervals: [],
},
};
const requestQuery = getRequestQuery( options );
// Disable eslint rule requiring `stats` to be defined below because the next two if statements
// depend on `getReportStats` to have been called.
@ -318,19 +360,20 @@ export function getReportChartData( options ) {
const stats = getReportStats( endpoint, requestQuery );
if ( isResolving( 'getReportStats', [ endpoint, requestQuery ] ) ) {
return { ...response, isRequesting: true };
return reportChartDataResponses.requesting;
}
if ( getReportStatsError( endpoint, requestQuery ) ) {
return { ...response, isError: true };
return reportChartDataResponses.error;
}
if ( isReportDataEmpty( stats, endpoint ) ) {
return { ...response, isEmpty: true };
return reportChartDataResponses.empty;
}
const totals = ( stats && stats.data && stats.data.totals ) || null;
let intervals = ( stats && stats.data && stats.data.intervals ) || [];
let intervals =
( stats && stats.data && stats.data.intervals ) || EMPTY_ARRAY;
// If we have more than 100 results for this time period,
// we need to make additional requests to complete the response.
@ -363,9 +406,9 @@ export function getReportChartData( options ) {
}
if ( isFetching ) {
return { ...response, isRequesting: true };
return reportChartDataResponses.requesting;
} else if ( isError ) {
return { ...response, isError: true };
return reportChartDataResponses.error;
}
forEach( pagedData, function ( _data ) {
@ -379,7 +422,11 @@ export function getReportChartData( options ) {
} );
}
return { ...response, data: { totals, intervals } };
return getReportChartDataResponse(
getResourceName( endpoint, requestQuery ),
totals,
intervals
);
}
/**

View File

@ -2,7 +2,7 @@
* External dependencies
*/
import moment from 'moment';
import { find } from 'lodash';
import { find, memoize } from 'lodash';
import { __ } from '@wordpress/i18n';
import { parse } from 'qs';
@ -208,50 +208,96 @@ export function getCurrentPeriod( period, compare ) {
* @param {Object} [before] - before date if custom period
* @return {DateValue} - DateValue data about the selected period
*/
function getDateValue( period, compare, after, before ) {
switch ( period ) {
case 'today':
return getCurrentPeriod( 'day', compare );
case 'yesterday':
return getLastPeriod( 'day', compare );
case 'week':
return getCurrentPeriod( 'week', compare );
case 'last_week':
return getLastPeriod( 'week', compare );
case 'month':
return getCurrentPeriod( 'month', compare );
case 'last_month':
return getLastPeriod( 'month', compare );
case 'quarter':
return getCurrentPeriod( 'quarter', compare );
case 'last_quarter':
return getLastPeriod( 'quarter', compare );
case 'year':
return getCurrentPeriod( 'year', compare );
case 'last_year':
return getLastPeriod( 'year', compare );
case 'custom':
const difference = before.diff( after, 'days' );
if ( compare === 'previous_period' ) {
const secondaryEnd = after.clone().subtract( 1, 'days' );
const secondaryStart = secondaryEnd
.clone()
.subtract( difference, 'days' );
const getDateValue = memoize(
( period, compare, after, before ) => {
switch ( period ) {
case 'today':
return getCurrentPeriod( 'day', compare );
case 'yesterday':
return getLastPeriod( 'day', compare );
case 'week':
return getCurrentPeriod( 'week', compare );
case 'last_week':
return getLastPeriod( 'week', compare );
case 'month':
return getCurrentPeriod( 'month', compare );
case 'last_month':
return getLastPeriod( 'month', compare );
case 'quarter':
return getCurrentPeriod( 'quarter', compare );
case 'last_quarter':
return getLastPeriod( 'quarter', compare );
case 'year':
return getCurrentPeriod( 'year', compare );
case 'last_year':
return getLastPeriod( 'year', compare );
case 'custom':
const difference = before.diff( after, 'days' );
if ( compare === 'previous_period' ) {
const secondaryEnd = after.clone().subtract( 1, 'days' );
const secondaryStart = secondaryEnd
.clone()
.subtract( difference, 'days' );
return {
primaryStart: after,
primaryEnd: before,
secondaryStart,
secondaryEnd,
};
}
return {
primaryStart: after,
primaryEnd: before,
secondaryStart,
secondaryEnd,
secondaryStart: after.clone().subtract( 1, 'years' ),
secondaryEnd: before.clone().subtract( 1, 'years' ),
};
}
}
},
( period, compare, after, before ) =>
[
period,
compare,
after && after.format(),
before && before.format(),
].join( ':' )
);
/**
* Memoized internal logic of getDateParamsFromQuery().
*
* @param {string} period - period value, ie `last_week`
* @param {string} compare - compare value, ie `previous_year`
* @param {string} after - date in iso date format, ie `2018-07-03`
* @param {string} before - date in iso date format, ie `2018-07-03`
* @param {string} defaultDateRange - the store's default date range
* @return {Object} - date parameters derived from query parameters with added defaults
*/
const getDateParamsFromQueryMemoized = memoize(
( period, compare, after, before, defaultDateRange ) => {
if ( period && compare ) {
return {
primaryStart: after,
primaryEnd: before,
secondaryStart: after.clone().subtract( 1, 'years' ),
secondaryEnd: before.clone().subtract( 1, 'years' ),
period,
compare,
after: after ? moment( after ) : null,
before: before ? moment( before ) : null,
};
}
}
}
const queryDefaults = parse(
defaultDateRange.replace( /&/g, '&' )
);
return {
period: queryDefaults.period,
compare: queryDefaults.compare,
after: queryDefaults.after ? moment( queryDefaults.after ) : null,
before: queryDefaults.before
? moment( queryDefaults.before )
: null,
};
},
( period, compare, after, before, defaultDateRange ) =>
[ period, compare, after, before, defaultDateRange ].join( ':' )
);
/**
* Add default date-related parameters to a query object
@ -269,23 +315,67 @@ export const getDateParamsFromQuery = (
defaultDateRange = 'period=month&compare=previous_year'
) => {
const { period, compare, after, before } = query;
if ( period && compare ) {
return {
return getDateParamsFromQueryMemoized(
period,
compare,
after,
before,
defaultDateRange
);
};
/**
* Memoized internal logic of getCurrentDates().
*
* @param {string} period - period value, ie `last_week`
* @param {string} compare - compare value, ie `previous_year`
* @param {Object} primaryStart - primary query start DateTime, in Moment instance.
* @param {Object} primaryEnd - primary query start DateTime, in Moment instance.
* @param {Object} secondaryStart - primary query start DateTime, in Moment instance.
* @param {Object} secondaryEnd - primary query start DateTime, in Moment instance.
* @return {{primary: DateValue, secondary: DateValue}} - Primary and secondary DateValue objects
*/
const getCurrentDatesMemoized = memoize(
(
period,
compare,
primaryStart,
primaryEnd,
secondaryStart,
secondaryEnd
) => ( {
primary: {
label: find( presetValues, ( item ) => item.value === period )
.label,
range: getRangeLabel( primaryStart, primaryEnd ),
after: primaryStart,
before: primaryEnd,
},
secondary: {
label: find( periods, ( item ) => item.value === compare ).label,
range: getRangeLabel( secondaryStart, secondaryEnd ),
after: secondaryStart,
before: secondaryEnd,
},
} ),
(
period,
compare,
primaryStart,
primaryEnd,
secondaryStart,
secondaryEnd
) =>
[
period,
compare,
after: after ? moment( after ) : null,
before: before ? moment( before ) : null,
};
}
const queryDefaults = parse( defaultDateRange.replace( /&/g, '&' ) );
return {
period: queryDefaults.period,
compare: queryDefaults.compare,
after: queryDefaults.after ? moment( queryDefaults.after ) : null,
before: queryDefaults.before ? moment( queryDefaults.before ) : null,
};
};
primaryStart && primaryStart.format(),
primaryEnd && primaryEnd.format(),
secondaryStart && secondaryStart.format(),
secondaryEnd && secondaryEnd.format(),
].join( ':' )
);
/**
* Get Date Value Objects for a primary and secondary date range
@ -313,21 +403,14 @@ export const getCurrentDates = (
secondaryEnd,
} = getDateValue( period, compare, after, before );
return {
primary: {
label: find( presetValues, ( item ) => item.value === period )
.label,
range: getRangeLabel( primaryStart, primaryEnd ),
after: primaryStart,
before: primaryEnd,
},
secondary: {
label: find( periods, ( item ) => item.value === compare ).label,
range: getRangeLabel( secondaryStart, secondaryEnd ),
after: secondaryStart,
before: secondaryEnd,
},
};
return getCurrentDatesMemoized(
period,
compare,
primaryStart,
primaryEnd,
secondaryStart,
secondaryEnd
);
};
/**