Skip to content

Fix: fix simple SPA layout animations#3516

Closed
MrVoshel wants to merge 12 commits intomotiondivision:mainfrom
MrVoshel:main
Closed

Fix: fix simple SPA layout animations#3516
MrVoshel wants to merge 12 commits intomotiondivision:mainfrom
MrVoshel:main

Conversation

@MrVoshel
Copy link

Avoid stale shared layout nodes during SPA navigations and add an HTML regression fixture for disconnected resumeFrom cases.

I encountered this problem on my SvelteKit app, but I suppose it affects other frameworks that approach navigation in a similar way as well. Svelte provides an onDestroy utility, but I wasn't able to get it to clean old nodes after the first navigation, only a full page refresh "fixed" it.

I suppose the additional checks shouldn't cause a perf regression since it happens only when adding or promoting, but I'm not very familiar with the codebase.

Please check carefully the logic and the test since I've narrowed down the issue thanks to Opus and GPT Codex, worked fine on my codebase, but haven't done anything extensive or stress tests.

Before the fix:

before_fix.mp4

After the fix:

after_fix.mp4

Avoid stale shared layout nodes during SPA navigations and add an HTML
regression fixture for disconnected resumeFrom cases.
Keep exiting nodes with snapshots while filtering stale layout stack members
and update the SPA regression fixture accordingly.
mattgperry pushed a commit that referenced this pull request Feb 2, 2026
Clean up disconnected DOM instances from the layout stack lazily when new
nodes are added. This prevents stale nodes from being used as resumeFrom
targets during shared element transitions.

The fix is minimal:
1. In add(): filter out members with disconnected instances and clear
   lead/prevLead if they were removed
2. In promote(): only use prevLead if its instance is still connected

This addresses the same issue as #3516 but with ~15 lines instead of 190+.

https://claude.ai/code/session_019kNt63LG4sBKvk5m87JVUb
@MrVoshel MrVoshel marked this pull request as draft February 3, 2026 13:09
precedent commits that tried to simplify the overall logic broke the stale SPA navigation on SvelteKit locally, so I'm trying with a different approach.

The test should probably be revised.
@MrVoshel MrVoshel marked this pull request as ready for review February 3, 2026 15:28
Cypress might not like ".at(index)", let's see if this passes all the tests. Behaviour in my sveltekit example seem to be correct again.
@MrVoshel
Copy link
Author

MrVoshel commented Feb 3, 2026

Clueless on why it fails on CircleCI
image

EDIT: It was probably because of their infra issues

the precedent logic was too flake, commit 03f8b1b in this PR should have failed. Tried making it more extensive (could also be split up is multiple more granular tests if the scope seems too big)
@mattgperry
Copy link
Collaborator

Closed by #3543

@mattgperry mattgperry closed this Feb 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants