test: re-render identity preservation tests for ArrayView + React#5619
Open
Karavil wants to merge 6 commits intorocicorp:mainfrom
Open
test: re-render identity preservation tests for ArrayView + React#5619Karavil wants to merge 6 commits intorocicorp:mainfrom
Karavil wants to merge 6 commits intorocicorp:mainfrom
Conversation
Adds comprehensive tests verifying that the immutable applyChange pipeline preserves object identity for unchanged nodes (enabling React.memo) while correctly bubbling new references up for changed descendants. NOTE: Several tests fail on main due to expandNode eagerly materializing all relationships (breaking identity). These pass with PR rocicorp#5605 which removes expandNode. The two PRs should be merged together. * ArrayView flat list: edit/add/remove preserve sibling identity * Relationship propagation: child edit/add/remove bubble new refs to parent * 3-level deep: grandchild edit bubbles through entire ancestor chain * React snapshot identity: getSnapshot stability, sentinel reuse * React.memo render counting: only changed row children re-render * Data flash prevention: no empty snapshots between data updates
|
Someone is attempting to deploy a commit to the Rocicorp Team on Vercel. A member of the Team first needs to authorize it. |
added 5 commits
February 26, 2026 12:05
* Add ASCII tree diagrams to every test explaining before/after * Add singular relationship identity test (.one() queries) * Add legitimate empty transition test (server returns empty then data) * Fix batched edit test to track refB and assert new reference * Extract shared types for readability in grandchild test
Uses QueryDelegateImpl with the test schema (issue → owner, comments) to build a real Source → Join → ArrayView pipeline, wires it to useSyncExternalStore, renders React.memo components, then edits a comment and verifies only the affected issue's component re-renders.
* Pass callGot: true to QueryDelegateImpl so the query reaches 'complete' result type (production-like lifecycle) * Move view.destroy() into afterEach so cleanup runs even if assertions fail mid-test
* Extract emit() helper to replace getListeners().forEach() pattern * Extract snapData/snapLength helpers for snapshot casting * Merge mock and integration React tests into single describe with shared DOM lifecycle and cleanups array * Inline MockView creation into newMockZero (only consumer) * Remove unnecessary unique counter
501a481 to
8d61b90
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds tests verifying that the immutable
applyChangepipeline preserves object identity for unchanged nodes (enablingReact.memo) while correctly bubbling new references up for changed descendants.6 of these tests fail on
main. This is intentional. They document a regression introduced byexpandNodein the change buffering PR. PR #5605 fixes all failures by removingexpandNode. Merge both PRs together.Test results on
main🔴 Failing: identity broken by
expandNodeeager materializationexpandNoderecursively callsArray.from(skipYields(thunk()), expandNode)on every relationship, creating new objects for every node in the tree on every push. This destroys reference identity for all relationship data, meaningReact.memocan never skip re-renders for components receiving rows with relationships.expandNodeallocates new objects for all nodes, not just the edited one.one()relationship child gets new ref from expansion🟢 Passing: no relationship expansion involved
expandNodedoesn't interfere🟢 Passing: React layer (ViewWrapper, not ArrayView)
These use mock views, so they test the
ViewWrapper/useSyncExternalStorecontract independently of the IVM pipeline.useSyncExternalStorecontract: no spurious re-rendersReact.memochild skips🟢 Passing: end-to-end integration (real IVM to React.memo)
Uses
QueryDelegateImplwith the test schema (issue→owner,comments) to build a realSource→Join→ArrayViewpipeline, wires it directly touseSyncExternalStore, rendersReact.memocomponents, then edits a comment and verifies only the affected issue's component re-renders.Test plan
fix/remove-eager-expand-nodebranch (perf: remove eager expandNode from ArrayView #5605)main(documenting the regression)array-view.test.tsandview-apply-change.test.tsstill pass