Fix optimistic rendering when deleting multiple records #9198
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This pull request modifies the optimistic rendering behavior when deleting multiple records in the Apollo cache management system.
- Replaces array splice operations with filter in
triggerUpdateRecordOptimisticEffectByBatch.tsfor more predictable record removal - Changes record existence check from
findIndex > -1tofind + isDefinedfor clearer intent - Improves edge case handling when removing records from the Apollo cache edges array
- Maintains consistent behavior with single record deletion through
triggerUpdateRecordOptimisticEffect.ts
The changes make the code more functional and less prone to mutation-related bugs, though with potential performance tradeoffs for large datasets.
1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
| rootQueryNextEdges = rootQueryNextEdges.filter( | ||
| (cachedEdge) => | ||
| readField('id', cachedEdge.node) !== updatedRecord.id, | ||
| ); |
There was a problem hiding this comment.
style: filter creates a new array on every iteration - for large deletions this could impact performance. Consider collecting IDs to remove first, then filtering once
| @@ -105,7 +103,10 @@ export const triggerUpdateRecordOptimisticEffectByBatch = ({ | |||
| } | |||
|
|
|||
| if (updatedRecordShouldBeRemovedFromRootQueryEdges) { | |||
| rootQueryNextEdges.splice(updatedRecordIndexInRootQueryEdges, 1); | |||
| rootQueryNextEdges = rootQueryNextEdges.filter( | |||
| (cachedEdge) => | |||
There was a problem hiding this comment.
splicing was causing an issue here because it modified the indexes of elements in rootQueryNextEdges, which was problematic when there were multiple iterations
Fixes https://discord.com/channels/1130383047699738754/1319676302944370730/1319676302944370730