Skip to content

Commit

Permalink
fix: memory leak related to re-reselect cache (opensearch-project#1201)
Browse files Browse the repository at this point in the history
  • Loading branch information
nickofthyme authored Jun 16, 2021
1 parent ecbcc1e commit 8cb6876
Show file tree
Hide file tree
Showing 126 changed files with 533 additions and 655 deletions.
10 changes: 9 additions & 1 deletion packages/osd-charts/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ module.exports = {
'import/no-cycle': [0, { maxDepth: 3, ignoreExternal: true }], // TODO: should error when this is fixed https://github.com/benmosher/eslint-plugin-import/issues/1453
'no-use-before-define': 0,
'no-restricted-properties': 0, // need to find and filter desired options
'class-methods-use-this': 1,
'class-methods-use-this': 0,
'unicorn/prefer-number-properties': 0,
'global-require': 1,
'import/no-dynamic-require': 1,
Expand Down Expand Up @@ -360,6 +360,14 @@ module.exports = {
]
: 0,

'no-restricted-imports': [
'error',
{
name: 're-reselect',
importNames: ['default'],
message: 'Please use `createCustomCachedSelector` instead.',
},
],
'no-underscore-dangle': 2,
'import/no-unresolved': 'error',
'import/no-extraneous-dependencies': 2,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,10 @@
* under the License.
*/

import createCachedSelector from 're-reselect';

import { ChartType } from '../../..';
import { SpecType } from '../../../../specs/constants';
import { GlobalChartState } from '../../../../state/chart_state';
import { getChartIdSelector } from '../../../../state/selectors/get_chart_id';
import { createCustomCachedSelector } from '../../../../state/create_selector';
import { getSpecs } from '../../../../state/selectors/get_settings_specs';
import { getSpecsFromStore } from '../../../../state/utils';
import { nullShapeViewModel, ShapeViewModel } from '../../layout/types/viewmodel_types';
Expand All @@ -32,10 +30,10 @@ import { render } from './scenegraph';
const getParentDimensions = (state: GlobalChartState) => state.parentDimensions;

/** @internal */
export const geometries = createCachedSelector(
export const geometries = createCustomCachedSelector(
[getSpecs, getParentDimensions],
(specs, parentDimensions): ShapeViewModel => {
const goalSpecs = getSpecsFromStore<GoalSpec>(specs, ChartType.Goal, SpecType.Series);
return goalSpecs.length === 1 ? render(goalSpecs[0], parentDimensions) : nullShapeViewModel();
},
)(getChartIdSelector);
);
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,10 @@
* under the License.
*/

import createCachedSelector from 're-reselect';

import { getChartIdSelector } from '../../../../state/selectors/get_chart_id';
import { createCustomCachedSelector } from '../../../../state/create_selector';
import { getSpecOrNull } from './goal_spec';

/** @internal */
export const getChartTypeDescriptionSelector = createCachedSelector([getSpecOrNull], (spec) => {
export const getChartTypeDescriptionSelector = createCustomCachedSelector([getSpecOrNull], (spec) => {
return `${spec?.subtype ?? 'goal'} chart`;
})(getChartIdSelector);
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,19 @@
* under the License.
*/

import createCachedSelector from 're-reselect';

import { getTooltipType } from '../../../../specs';
import { TooltipType } from '../../../../specs/constants';
import { getChartIdSelector } from '../../../../state/selectors/get_chart_id';
import { createCustomCachedSelector } from '../../../../state/create_selector';
import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs';
import { getTooltipInfoSelector } from './tooltip';

/** @internal */
export const isTooltipVisibleSelector = createCachedSelector(
export const isTooltipVisibleSelector = createCustomCachedSelector(
[getSettingsSpecSelector, getTooltipInfoSelector],
(settingsSpec, tooltipInfo): boolean => {
if (getTooltipType(settingsSpec) === TooltipType.None) {
return false;
}
return tooltipInfo.values.length > 0;
},
)(getChartIdSelector);
);
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,12 @@
* under the License.
*/

import createCachedSelector from 're-reselect';
import { Selector } from 'reselect';

import { ChartType } from '../../..';
import { getOnElementClickSelector } from '../../../../common/event_handler_selectors';
import { GlobalChartState, PointerStates } from '../../../../state/chart_state';
import { getChartIdSelector } from '../../../../state/selectors/get_chart_id';
import { createCustomCachedSelector } from '../../../../state/create_selector';
import { getLastClickSelector } from '../../../../state/selectors/get_last_click';
import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs';
import { getSpecOrNull } from './goal_spec';
Expand All @@ -41,10 +40,10 @@ export function createOnElementClickCaller(): (state: GlobalChartState) => void
let selector: Selector<GlobalChartState, void> | null = null;
return (state: GlobalChartState) => {
if (selector === null && state.chartType === ChartType.Goal) {
selector = createCachedSelector(
selector = createCustomCachedSelector(
[getSpecOrNull, getLastClickSelector, getSettingsSpecSelector, getPickedShapesLayerValues],
getOnElementClickSelector(prev),
)(getChartIdSelector);
);
}
if (selector) {
selector(state);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,12 @@
* under the License.
*/

import createCachedSelector from 're-reselect';
import { Selector } from 'react-redux';

import { ChartType } from '../../..';
import { getOnElementOutSelector } from '../../../../common/event_handler_selectors';
import { GlobalChartState } from '../../../../state/chart_state';
import { getChartIdSelector } from '../../../../state/selectors/get_chart_id';
import { createCustomCachedSelector } from '../../../../state/create_selector';
import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs';
import { getSpecOrNull } from './goal_spec';
import { getPickedShapesLayerValues } from './picked_shapes';
Expand All @@ -39,10 +38,10 @@ export function createOnElementOutCaller(): (state: GlobalChartState) => void {
let selector: Selector<GlobalChartState, void> | null = null;
return (state: GlobalChartState) => {
if (selector === null && state.chartType === ChartType.Goal) {
selector = createCachedSelector(
selector = createCustomCachedSelector(
[getSpecOrNull, getPickedShapesLayerValues, getSettingsSpecSelector],
getOnElementOutSelector(prev),
)(getChartIdSelector);
);
}
if (selector) {
selector(state);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,13 @@
* under the License.
*/

import createCachedSelector from 're-reselect';
import { Selector } from 'react-redux';

import { ChartType } from '../../..';
import { getOnElementOverSelector } from '../../../../common/event_handler_selectors';
import { LayerValue } from '../../../../specs';
import { GlobalChartState } from '../../../../state/chart_state';
import { getChartIdSelector } from '../../../../state/selectors/get_chart_id';
import { createCustomCachedSelector } from '../../../../state/create_selector';
import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs';
import { getSpecOrNull } from './goal_spec';
import { getPickedShapesLayerValues } from './picked_shapes';
Expand All @@ -40,12 +39,10 @@ export function createOnElementOverCaller(): (state: GlobalChartState) => void {
let selector: Selector<GlobalChartState, void> | null = null;
return (state: GlobalChartState) => {
if (selector === null && state.chartType === ChartType.Goal) {
selector = createCachedSelector(
selector = createCustomCachedSelector(
[getSpecOrNull, getPickedShapesLayerValues, getSettingsSpecSelector],
getOnElementOverSelector(prev),
)({
keySelector: getChartIdSelector,
});
);
}
if (selector) {
selector(state);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,9 @@
* under the License.
*/

import createCachedSelector from 're-reselect';

import { LayerValue } from '../../../../specs';
import { GlobalChartState } from '../../../../state/chart_state';
import { getChartIdSelector } from '../../../../state/selectors/get_chart_id';
import { createCustomCachedSelector } from '../../../../state/create_selector';
import { BulletViewModel } from '../../layout/types/viewmodel_types';
import { geometries } from './geometries';

Expand All @@ -30,7 +28,7 @@ function getCurrentPointerPosition(state: GlobalChartState) {
}

/** @internal */
export const getPickedShapes = createCachedSelector(
export const getPickedShapes = createCustomCachedSelector(
[geometries, getCurrentPointerPosition],
(geoms, pointerPosition): BulletViewModel[] => {
const picker = geoms.pickQuads;
Expand All @@ -39,10 +37,10 @@ export const getPickedShapes = createCachedSelector(
const y = pointerPosition.y - chartCenter.y;
return picker(x, y);
},
)(getChartIdSelector);
);

/** @internal */
export const getPickedShapesLayerValues = createCachedSelector(
export const getPickedShapesLayerValues = createCustomCachedSelector(
[getPickedShapes],
(pickedShapes): Array<Array<LayerValue>> => {
const elements = pickedShapes.map<Array<LayerValue>>((model) => {
Expand All @@ -59,4 +57,4 @@ export const getPickedShapesLayerValues = createCachedSelector(
});
return elements;
},
)(getChartIdSelector);
);
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,8 @@
* under the License.
*/

import createCachedSelector from 're-reselect';

import { TooltipInfo } from '../../../../components/tooltip/types';
import { getChartIdSelector } from '../../../../state/selectors/get_chart_id';
import { createCustomCachedSelector } from '../../../../state/create_selector';
import { getSpecOrNull } from './goal_spec';
import { getPickedShapes } from './picked_shapes';

Expand All @@ -30,7 +28,7 @@ const EMPTY_TOOLTIP = Object.freeze({
});

/** @internal */
export const getTooltipInfoSelector = createCachedSelector(
export const getTooltipInfoSelector = createCustomCachedSelector(
[getSpecOrNull, getPickedShapes],
(spec, pickedShapes): TooltipInfo => {
if (!spec) {
Expand Down Expand Up @@ -60,4 +58,4 @@ export const getTooltipInfoSelector = createCachedSelector(

return tooltipInfo;
},
)(getChartIdSelector);
);
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,10 @@
*/

import { max as d3Max } from 'd3-array';
import createCachedSelector from 're-reselect';

import { Box, measureText } from '../../../../common/text_utils';
import { GlobalChartState } from '../../../../state/chart_state';
import { getChartIdSelector } from '../../../../state/selectors/get_chart_id';
import { createCustomCachedSelector } from '../../../../state/create_selector';
import { getLegendSizeSelector } from '../../../../state/selectors/get_legend_size';
import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs';
import { Position } from '../../../../utils/common';
Expand Down Expand Up @@ -50,7 +49,7 @@ const getParentDimension = (state: GlobalChartState) => state.parentDimensions;
* Gets charts grid area excluding legend and X,Y axis labels and paddings.
* @internal
*/
export const computeChartDimensionsSelector = createCachedSelector(
export const computeChartDimensionsSelector = createCustomCachedSelector(
[
getParentDimension,
getLegendSizeSelector,
Expand Down Expand Up @@ -114,4 +113,4 @@ export const computeChartDimensionsSelector = createCachedSelector(
left,
};
},
)(getChartIdSelector);
);
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,14 @@
* under the License.
*/

import createCachedSelector from 're-reselect';

import { LegendItem } from '../../../../common/legend';
import { getChartIdSelector } from '../../../../state/selectors/get_chart_id';
import { createCustomCachedSelector } from '../../../../state/create_selector';
import { getDeselectedSeriesSelector } from '../../../../state/selectors/get_deselected_data_series';
import { getColorScale } from './get_color_scale';
import { getSpecOrNull } from './heatmap_spec';

/** @internal */
export const computeLegendSelector = createCachedSelector(
export const computeLegendSelector = createCustomCachedSelector(
[getSpecOrNull, getColorScale, getDeselectedSeriesSelector],
(spec, colorScale, deselectedDataSeries): LegendItem[] => {
const legendItems: LegendItem[] = [];
Expand All @@ -53,4 +51,4 @@ export const computeLegendSelector = createCachedSelector(
};
});
},
)(getChartIdSelector);
);
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,8 @@
* under the License.
*/

import createCachedSelector from 're-reselect';

import { GlobalChartState } from '../../../../state/chart_state';
import { getChartIdSelector } from '../../../../state/selectors/get_chart_id';
import { createCustomCachedSelector } from '../../../../state/create_selector';
import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs';
import { nullShapeViewModel, ShapeViewModel } from '../../layout/types/viewmodel_types';
import { computeChartDimensionsSelector } from './compute_chart_dimensions';
Expand All @@ -34,7 +32,7 @@ import { render } from './scenegraph';
const getDeselectedSeriesSelector = (state: GlobalChartState) => state.interactions.deselectedDataSeries;

/** @internal */
export const geometries = createCachedSelector(
export const geometries = createCustomCachedSelector(
[
getHeatmapSpecSelector,
computeChartDimensionsSelector,
Expand Down Expand Up @@ -73,4 +71,4 @@ export const geometries = createCachedSelector(
? render(heatmapSpec, settingSpec, chartDimensions, heatmapTable, colorScale, ranges, gridHeightParams)
: nullShapeViewModel();
},
)(getChartIdSelector);
);
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,9 @@
* under the License.
*/

import createCachedSelector from 're-reselect';

import { BrushAxis } from '../../../../specs';
import { GlobalChartState } from '../../../../state/chart_state';
import { getChartIdSelector } from '../../../../state/selectors/get_chart_id';
import { createCustomCachedSelector } from '../../../../state/create_selector';
import { getChartRotationSelector } from '../../../../state/selectors/get_chart_rotation';
import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs';
import { Dimensions } from '../../../../utils/dimensions';
Expand All @@ -32,7 +30,7 @@ const getIsDragging = (state: GlobalChartState) => state.interactions.pointer.dr
const getCurrentPointerPosition = (state: GlobalChartState) => state.interactions.pointer.current.position;

/** @internal */
export const getBrushAreaSelector = createCachedSelector(
export const getBrushAreaSelector = createCustomCachedSelector(
[
getIsDragging,
getMouseDownPosition,
Expand All @@ -55,4 +53,4 @@ export const getBrushAreaSelector = createCachedSelector(
return { top: start.y, left: start.x, width: end.x - start.x - chartDimensions.left, height: end.y - start.y };
}
},
)(getChartIdSelector);
);
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,8 @@
* under the License.
*/

import createCachedSelector from 're-reselect';

import { GlobalChartState } from '../../../../state/chart_state';
import { getChartIdSelector } from '../../../../state/selectors/get_chart_id';
import { createCustomCachedSelector } from '../../../../state/create_selector';
import { DragShape } from '../../layout/types/viewmodel_types';
import { geometries } from './geometries';

Expand All @@ -29,7 +27,7 @@ function getCurrentPointerStates(state: GlobalChartState) {
}

/** @internal */
export const getBrushedHighlightedShapesSelector = createCachedSelector(
export const getBrushedHighlightedShapesSelector = createCustomCachedSelector(
[geometries, getCurrentPointerStates],
(geoms, pointerStates): DragShape | null => {
if (!pointerStates.dragging || !pointerStates.down) {
Expand All @@ -51,4 +49,4 @@ export const getBrushedHighlightedShapesSelector = createCachedSelector(
]);
return shape;
},
)(getChartIdSelector);
);
Loading

0 comments on commit 8cb6876

Please sign in to comment.