From 3f9a4f4421339fc752cf718de0c2f8ac94f6d1b4 Mon Sep 17 00:00:00 2001 From: Talha Naqvi Date: Mon, 21 Mar 2022 10:40:42 -0700 Subject: [PATCH] Fix stable id collisions and provide alternate for optimiseForInsertDeleteAnimations (#696) * clearing stable id map to avoid collisions * Fixes stable id and introduces new API for layout animations * Added warning if animations render is requested on pagination * updated docs Co-authored-by: Talha Naqvi --- README.md | 1 - src/core/RecyclerListView.tsx | 20 ++++-- src/core/VirtualRenderer.ts | 87 ++++++++++++++++++------- src/core/constants/Messages.ts | 2 + src/core/layoutmanager/LayoutManager.ts | 13 ++++ 5 files changed, 93 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 3b90aa36..ff1b53a3 100644 --- a/README.md +++ b/README.md @@ -104,7 +104,6 @@ In case you cannot determine heights of items in advance just set `forceNonDeter | forceNonDeterministicRendering | No | boolean | Default is false; if enabled dimensions provided in layout provider will not be strictly enforced. Use this if item dimensions cannot be accurately determined | | extendedState | No | object | In some cases the data passed at row level may not contain all the info that the item depends upon, you can keep all other info outside and pass it down via this prop. Changing this object will cause everything to re-render. Make sure you don't change it often to ensure performance. Re-renders are heavy. | | itemAnimator | No | ItemAnimator | Enables animating RecyclerListView item cells (shift, add, remove, etc) | -| optimizeForInsertDeleteAnimations | No | boolean | Enables you to utilize layout animations better by unmounting removed items | | style | No | object | To pass down style to inner ScrollView | | scrollViewProps | No | object | For all props that need to be proxied to inner/external scrollview. Put them in an object and they'll be spread and passed down. | | layoutSize | No | Dimension | Will prevent the initial empty render required to compute the size of the listview and use these dimensions to render list items in the first render itself. This is useful for cases such as server side rendering. The prop canChangeSize has to be set to true if the size can be changed after rendering. Note that this is not the scroll view size and is used solely for layouting. | diff --git a/src/core/RecyclerListView.tsx b/src/core/RecyclerListView.tsx index 32c965de..057fa6c9 100644 --- a/src/core/RecyclerListView.tsx +++ b/src/core/RecyclerListView.tsx @@ -208,6 +208,7 @@ export default class RecyclerListView

void, scrollOnNextUpdate: (point: Point) => void, @@ -86,6 +87,14 @@ export default class VirtualRenderer { return { height: 0, width: 0 }; } + public setOptimizeForAnimations(shouldOptimize: boolean): void { + this._optimizeForAnimations = shouldOptimize; + } + + public hasPendingAnimationOptimization(): boolean { + return this._optimizeForAnimations; + } + public updateOffset(offsetX: number, offsetY: number, isActual: boolean, correction: WindowCorrection): void { if (this._viewabilityTracker) { const offset = this._params && this._params.isHorizontal ? offsetX : offsetY; @@ -203,7 +212,9 @@ export default class VirtualRenderer { } } - public syncAndGetKey(index: number, overrideStableIdProvider?: StableIdProvider, newRenderStack?: RenderStack): string { + public syncAndGetKey(index: number, overrideStableIdProvider?: StableIdProvider, + newRenderStack?: RenderStack, + keyToStableIdMap?: { [key: string]: string } ): string { const getStableId = overrideStableIdProvider ? overrideStableIdProvider : this._fetchStableId; const renderStack = newRenderStack ? newRenderStack : this._renderStack; const stableIdItem = this._stableIdToRenderKeyMap[getStableId(index)]; @@ -222,6 +233,9 @@ export default class VirtualRenderer { } } else { renderStack[key] = { dataIndex: index }; + if (keyToStableIdMap && keyToStableIdMap[key]) { + delete this._stableIdToRenderKeyMap[keyToStableIdMap[key]]; + } } } else { key = getStableId(index); @@ -248,11 +262,18 @@ export default class VirtualRenderer { } //Further optimize in later revision, pretty fast for now considering this is a low frequency event - public handleDataSetChange(newDataProvider: BaseDataProvider, shouldOptimizeForAnimations?: boolean): void { + public handleDataSetChange(newDataProvider: BaseDataProvider): void { const getStableId = newDataProvider.getStableId; const maxIndex = newDataProvider.getSize() - 1; const activeStableIds: { [key: string]: number } = {}; const newRenderStack: RenderStack = {}; + const keyToStableIdMap: { [key: string]: string } = {}; + + // Do not use recycle pool so that elements don't fly top to bottom or vice version + // Doing this is expensive and can draw extra items + if (this._optimizeForAnimations) { + this._recyclePool.clearAll(); + } //Compute active stable ids and stale active keys and resync render stack for (const key in this._renderStack) { @@ -272,38 +293,54 @@ export default class VirtualRenderer { const oldActiveStableIdsCount = oldActiveStableIds.length; for (let i = 0; i < oldActiveStableIdsCount; i++) { const key = oldActiveStableIds[i]; - if (!activeStableIds[key]) { - if (!shouldOptimizeForAnimations && this._isRecyclingEnabled) { - const stableIdItem = this._stableIdToRenderKeyMap[key]; - if (stableIdItem) { + const stableIdItem = this._stableIdToRenderKeyMap[key]; + if (stableIdItem) { + if (!activeStableIds[key]) { + if (!this._optimizeForAnimations && this._isRecyclingEnabled) { this._recyclePool.putRecycledObject(stableIdItem.type, stableIdItem.key); } + delete this._stableIdToRenderKeyMap[key]; + + const stackItem = this._renderStack[stableIdItem.key]; + const dataIndex = stackItem ? stackItem.dataIndex : undefined; + if (!ObjectUtil.isNullOrUndefined(dataIndex) && dataIndex <= maxIndex && this._layoutManager) { + this._layoutManager.removeLayout(dataIndex); + } + } else { + keyToStableIdMap[stableIdItem.key] = key; } - delete this._stableIdToRenderKeyMap[key]; } } - - for (const key in this._renderStack) { - if (this._renderStack.hasOwnProperty(key)) { - const index = this._renderStack[key].dataIndex; - if (!ObjectUtil.isNullOrUndefined(index)) { - if (index <= maxIndex) { - const newKey = this.syncAndGetKey(index, getStableId, newRenderStack); - const newStackItem = newRenderStack[newKey]; - if (!newStackItem) { - newRenderStack[newKey] = { dataIndex: index }; - } else if (newStackItem.dataIndex !== index) { - const cllKey = this._getCollisionAvoidingKey(); - newRenderStack[cllKey] = { dataIndex: index }; - this._stableIdToRenderKeyMap[getStableId(index)] = { - key: cllKey, type: this._layoutProvider.getLayoutTypeForIndex(index), - }; - } + const renderStackKeys = Object.keys(this._renderStack).sort((a, b) => { + const firstItem = this._renderStack[a]; + const secondItem = this._renderStack[b]; + if (firstItem && firstItem.dataIndex && secondItem && secondItem.dataIndex) { + return firstItem.dataIndex - secondItem.dataIndex; + } + return 1; + }); + const renderStackLength = renderStackKeys.length; + for (let i = 0; i < renderStackLength; i++) { + const key = renderStackKeys[i]; + const index = this._renderStack[key].dataIndex; + if (!ObjectUtil.isNullOrUndefined(index)) { + if (index <= maxIndex) { + const newKey = this.syncAndGetKey(index, getStableId, newRenderStack, keyToStableIdMap); + const newStackItem = newRenderStack[newKey]; + if (!newStackItem) { + newRenderStack[newKey] = { dataIndex: index }; + } else if (newStackItem.dataIndex !== index) { + const cllKey = this._getCollisionAvoidingKey(); + newRenderStack[cllKey] = { dataIndex: index }; + this._stableIdToRenderKeyMap[getStableId(index)] = { + key: cllKey, type: this._layoutProvider.getLayoutTypeForIndex(index), + }; } } - delete this._renderStack[key]; } + delete this._renderStack[key]; } + Object.assign(this._renderStack, newRenderStack); for (const key in this._renderStack) { diff --git a/src/core/constants/Messages.ts b/src/core/constants/Messages.ts index 6cb6004f..199af29f 100644 --- a/src/core/constants/Messages.ts +++ b/src/core/constants/Messages.ts @@ -4,4 +4,6 @@ export const Messages: {[key: string]: string} = { WARN_NO_DATA: "You have mounted RecyclerListView with an empty data provider (Size in 0). Please mount only if there is atleast one item " + "to ensure optimal performance and to avoid unexpected behavior", VISIBLE_INDEXES_CHANGED_DEPRECATED: "onVisibleIndexesChanged deprecated. Please use onVisibleIndicesChanged instead.", + ANIMATION_ON_PAGINATION: "Looks like you're trying to use RecyclerListView's layout animation render while doing pagination. " + + "This operation will be ignored to avoid creation of too many items due to developer error.", }; diff --git a/src/core/layoutmanager/LayoutManager.ts b/src/core/layoutmanager/LayoutManager.ts index e9454a4a..72726417 100644 --- a/src/core/layoutmanager/LayoutManager.ts +++ b/src/core/layoutmanager/LayoutManager.ts @@ -23,6 +23,19 @@ export abstract class LayoutManager { return undefined; } + //Removes item at the specified index + public removeLayout(index: number): void { + const layouts = this.getLayouts(); + if (index < layouts.length) { + layouts.splice(index, 1); + } + if (index === 0 && layouts.length > 0) { + const firstLayout = layouts[0]; + firstLayout.x = 0; + firstLayout.y = 0; + } + } + //Return the dimension of entire content inside the list public abstract getContentDimension(): Dimension;