Fix stable id collisions and provide alternate for optimiseForInsertDeleteAnimations

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. |

src/core/RecyclerListView.tsx

@@ -208,6 +208,7 @@ export default class RecyclerListView<P extends RecyclerListViewProps, S extends
         if (this.props.dataProvider.getSize() === 0) {
             console.warn(Messages.WARN_NO_DATA); //tslint:disable-line
         }
+        this._virtualRenderer.setOptimizeForAnimations(false);
     }
 
     public componentDidMount(): void {
@@ -397,6 +398,14 @@ export default class RecyclerListView<P extends RecyclerListViewProps, S extends
         );
     }
 
+    // Disables recycling for the next frame so that layout animations run well.
+    // WARNING: Avoid this when making large changes to the data as the list might draw too much to run animations. Single item insertions/deletions
+    // should be good. With recycling paused the list cannot do much optimization.
+    // The next render will run as normal and reuse items.
+    public prepareForLayoutAnimationRender(): void {
+        this._virtualRenderer.setOptimizeForAnimations(true);
+    }
+
     protected getVirtualRenderer(): VirtualRenderer {
         return this._virtualRenderer;
     }
@@ -459,8 +468,12 @@ export default class RecyclerListView<P extends RecyclerListViewProps, S extends
         this._params.itemCount = newProps.dataProvider.getSize();
         this._virtualRenderer.setParamsAndDimensions(this._params, this._layout);
         this._virtualRenderer.setLayoutProvider(newProps.layoutProvider);
-        if (newProps.dataProvider.hasStableIds() && this.props.dataProvider !== newProps.dataProvider && newProps.dataProvider.requiresDataChangeHandling()) {
-            this._virtualRenderer.handleDataSetChange(newProps.dataProvider, this.props.optimizeForInsertDeleteAnimations);
+        if (newProps.dataProvider.hasStableIds() && this.props.dataProvider !== newProps.dataProvider) {
+            if (newProps.dataProvider.requiresDataChangeHandling()) {
+                this._virtualRenderer.handleDataSetChange(newProps.dataProvider);
+            } else if (this._virtualRenderer.hasPendingAnimationOptimization()) {
+                console.warn(Messages.ANIMATION_ON_PAGINATION); //tslint:disable-line
+            }
         }
         if (this.props.layoutProvider !== newProps.layoutProvider || this.props.isHorizontal !== newProps.isHorizontal) {
             //TODO:Talha use old layout manager
@@ -835,8 +848,7 @@ RecyclerListView.propTypes = {
     //This container is for wrapping individual cells that are being rendered by recyclerlistview unlike contentContainer which wraps all of them.
     renderItemContainer: PropTypes.func,
 
-    //Enables you to utilize layout animations better by unmounting removed items. Please note, this might increase unmounts
-    //on large data changes.
+    //Deprecated in favour of `prepareForLayoutAnimationRender` method
     optimizeForInsertDeleteAnimations: PropTypes.bool,
 
     //To pass down style to inner ScrollView

src/core/VirtualRenderer.ts

@@ -51,6 +51,7 @@ export default class VirtualRenderer {
     private _layoutManager: LayoutManager | null = null;
     private _viewabilityTracker: ViewabilityTracker | null = null;
     private _dimensions: Dimension | null;
+    private _optimizeForAnimations: boolean = false;
 
     constructor(renderStackChanged: (renderStack: RenderStack) => 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) {

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.",
 };

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;