Skip to content

Commit

Permalink
Fix stable id collisions and provide alternate for optimiseForInsertD…
Browse files Browse the repository at this point in the history 
…eleteAnimations (#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 <talha.naqvi@shopify.com>
  • Loading branch information
@naqvitalha
naqvitalha and naqvitalha committed Mar 21, 2022
1 parent 8032cab commit 3f9a4f4
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 30 deletions.
1 change: 0 additions & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Expand Down
20 changes: 16 additions & 4 deletions src/core/RecyclerListView.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
87 changes: 62 additions & 25 deletions src/core/VirtualRenderer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)];
Expand All @@ -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);
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down
2 changes: 2 additions & 0 deletions src/core/constants/Messages.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
};
13 changes: 13 additions & 0 deletions src/core/layoutmanager/LayoutManager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit 3f9a4f4

Please sign in to comment.