From 0a512adaf0ac9b0621fc2fc72c048fd962baf7a2 Mon Sep 17 00:00:00 2001 From: Jason Law Date: Sat, 30 Nov 2019 12:14:04 +0800 Subject: [PATCH 1/2] fix: memory leak fix (#1610) * fix: memory leak * Style change --- src/plugin-hooks/useExpanded.js | 10 +++++++++- src/plugin-hooks/useGroupBy.js | 10 +++++++++- src/plugin-hooks/useResizeColumns.js | 12 ++++++++++-- src/plugin-hooks/useRowSelect.js | 17 ++++++++++++----- src/plugin-hooks/useSortBy.js | 12 ++++++++++-- 5 files changed, 50 insertions(+), 11 deletions(-) diff --git a/src/plugin-hooks/useExpanded.js b/src/plugin-hooks/useExpanded.js index 967ad6b..f19479f 100755 --- a/src/plugin-hooks/useExpanded.js +++ b/src/plugin-hooks/useExpanded.js @@ -76,6 +76,10 @@ function useMain(instance) { }, actions.toggleExpanded) } + // use reference to avoid memory leak in #1608 + const instanceRef = React.useRef() + instanceRef.current = instance + hooks.prepareRow.push(row => { row.toggleExpanded = set => toggleExpandedByPath(row.path, set) row.getExpandedToggleProps = props => { @@ -90,7 +94,11 @@ function useMain(instance) { }, title: 'Toggle Expanded', }, - applyPropHooks(instance.hooks.getExpandedToggleProps, row, instance), + applyPropHooks( + instanceRef.current.hooks.getExpandedToggleProps, + row, + instanceRef.current + ), props ) } diff --git a/src/plugin-hooks/useGroupBy.js b/src/plugin-hooks/useGroupBy.js index ad51356..54a6413 100755 --- a/src/plugin-hooks/useGroupBy.js +++ b/src/plugin-hooks/useGroupBy.js @@ -107,6 +107,10 @@ function useMain(instance) { hooks.getGroupByToggleProps = [] + // use reference to avoid memory leak in #1608 + const instanceRef = React.useRef() + instanceRef.current = instance + flatHeaders.forEach(header => { const { canGroupBy } = header header.getGroupByToggleProps = props => { @@ -123,7 +127,11 @@ function useMain(instance) { }, title: 'Toggle GroupBy', }, - applyPropHooks(instance.hooks.getGroupByToggleProps, header, instance), + applyPropHooks( + instanceRef.current.hooks.getGroupByToggleProps, + header, + instanceRef.current + ), props ) } diff --git a/src/plugin-hooks/useResizeColumns.js b/src/plugin-hooks/useResizeColumns.js index 6aa21a6..061ecf0 100644 --- a/src/plugin-hooks/useResizeColumns.js +++ b/src/plugin-hooks/useResizeColumns.js @@ -1,4 +1,4 @@ -// +import React from 'react' import { defaultState } from '../hooks/useTable' import { defaultColumn, getFirstDefined } from '../utils' @@ -93,6 +93,10 @@ const useBeforeDimensions = instance => { })) } + // use reference to avoid memory leak in #1608 + const instanceRef = React.useRef() + instanceRef.current = instance + flatHeaders.forEach(header => { const canResize = getFirstDefined( header.disableResizing === true ? false : undefined, @@ -114,7 +118,11 @@ const useBeforeDimensions = instance => { }, draggable: false, }, - applyPropHooks(instance.hooks.getResizerProps, header, instance), + applyPropHooks( + instanceRef.current.hooks.getResizerProps, + header, + instanceRef.current + ), userProps ) } diff --git a/src/plugin-hooks/useRowSelect.js b/src/plugin-hooks/useRowSelect.js index d4b790e..5ec52aa 100644 --- a/src/plugin-hooks/useRowSelect.js +++ b/src/plugin-hooks/useRowSelect.js @@ -168,6 +168,10 @@ function useMain(instance) { }, actions.toggleRowSelected) } + // use reference to avoid memory leak in #1608 + const instanceRef = React.useRef() + instanceRef.current = instance + const getToggleAllRowsSelectedProps = props => { return mergeProps( { @@ -180,7 +184,10 @@ function useMain(instance) { checked: isAllRowsSelected, title: 'Toggle All Rows Selected', }, - applyPropHooks(instance.hooks.getToggleAllRowsSelectedProps, instance), + applyPropHooks( + instanceRef.current.hooks.getToggleAllRowsSelectedProps, + instanceRef.current + ), props ) } @@ -216,9 +223,9 @@ function useMain(instance) { title: 'Toggle Row Selected', }, applyPropHooks( - instance.hooks.getToggleRowSelectedProps, + instanceRef.current.hooks.getToggleRowSelectedProps, row, - instance + instanceRef.current ), props ) @@ -246,9 +253,9 @@ function useMain(instance) { title: 'Toggle Row Selected', }, applyPropHooks( - instance.hooks.getToggleRowSelectedProps, + instanceRef.current.hooks.getToggleRowSelectedProps, row, - instance + instanceRef.current ), props ) diff --git a/src/plugin-hooks/useSortBy.js b/src/plugin-hooks/useSortBy.js index b46f139..e04bfac 100755 --- a/src/plugin-hooks/useSortBy.js +++ b/src/plugin-hooks/useSortBy.js @@ -155,6 +155,10 @@ function useMain(instance) { }, actions.sortByChange) } + // use reference to avoid memory leak in #1608 + const instanceRef = React.useRef() + instanceRef.current = instance + // Add the getSortByToggleProps method to columns and headers flatHeaders.forEach(column => { const { @@ -198,7 +202,7 @@ function useMain(instance) { e.persist() column.toggleSortBy( undefined, - !instance.disableMultiSort && isMultiSortEvent(e) + !instanceRef.current.disableMultiSort && isMultiSortEvent(e) ) } : undefined, @@ -207,7 +211,11 @@ function useMain(instance) { }, title: 'Toggle SortBy', }, - applyPropHooks(instance.hooks.getSortByToggleProps, column, instance), + applyPropHooks( + instanceRef.current.hooks.getSortByToggleProps, + column, + instanceRef.current + ), props ) } From 764a8ce28187d44fb8db21650c54e7c5f9fab4f6 Mon Sep 17 00:00:00 2001 From: Andros Rosa Llop Date: Sat, 30 Nov 2019 00:14:25 -0400 Subject: [PATCH 2/2] Fix default resetSelectedRowsDeps (#1663) * This one also is listening to rows, instead of data. --- src/plugin-hooks/useExpanded.js | 2 +- src/plugin-hooks/useRowSelect.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugin-hooks/useExpanded.js b/src/plugin-hooks/useExpanded.js index f19479f..58b4971 100755 --- a/src/plugin-hooks/useExpanded.js +++ b/src/plugin-hooks/useExpanded.js @@ -20,7 +20,7 @@ export const useExpanded = hooks => { useExpanded.pluginName = 'useExpanded' -const defaultGetResetExpandedDeps = instance => [instance.data] +const defaultGetResetExpandedDeps = ({ data }) => [data] function useMain(instance) { const { diff --git a/src/plugin-hooks/useRowSelect.js b/src/plugin-hooks/useRowSelect.js index 5ec52aa..989d175 100644 --- a/src/plugin-hooks/useRowSelect.js +++ b/src/plugin-hooks/useRowSelect.js @@ -49,7 +49,7 @@ function useRows(rows, instance) { return rows } -const defaultGetResetSelectedRowPathsDeps = ({ rows }) => [rows] +const defaultGetResetSelectedRowPathsDeps = ({ data }) => [data] function useMain(instance) { const {