Sprite Lab in Lab2: fix level switch#73854
Open
breville wants to merge 3 commits into
Open
Conversation
The legacy pageConstants reducer forbids changing a key once set - it assumes one page load per level. Lab2 switches levels in place, so navigating between two SpriteLab2 levels threw "Can't change value of key 'channelId'" from the seeding effect and took down the page. The lab now registers the reducer wrapped: a change the legacy guard rejects is re-applied from the initial state (last write wins). Wrapping beats resetting on LifecycleEvent.LevelChangeRequested: the view remounts on level switch and re-seeds the stale channel before the new one loads, so no event-ordered reset can win that race. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
76cfb0e to
36e40fc
Compare
…es channel With useSources (#73868) the inner view unmounts while the next level's project loads and the channel arrives as a fresh prop, so the stale-channel re-seed race that forced the catch-and-retry wrapper is gone. The wrapper now handles an explicit lab-owned reset action instead, dispatched from the seeding effect's cleanup: React runs cleanup before the next effect, so every seed lands on a fresh slice, and the legacy change-guard stays enforced between seed cycles. A lifecycle-event reset would still race: the seed effect re-runs on progress.currentLevelId while the old view is briefly mounted, and would re-seed the old channel over an event-ordered reset. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
[written by Claude]
Navigating between two SpriteLab2 levels in a lesson crashed the page:
Error: Can't change value of key "channelId", then the error boundary.Root cause
SpriteLab2 seeds the legacy
pageConstantsslice (isBlockly,channelId, …) because it bypassesStudioApp.init, and the legacy reducer forbids changing a key once set — it assumes one page load per level, and offers no reset action. Lab2 switches levels in place: the second level seeds a differentchannelIdand the reducer throws mid-dispatch. Single-level sessions never hit it because the firstundefined → valuetransition is legal; it takes a second, different channel.The fix
The lab registers the reducer wrapped (
reseedablePageConstants): the wrapper handles a lab-ownedRESET_PAGE_CONSTANTSaction (returning the legacy initial state) and delegates everything else untouched, guard intact. The seeding effect dispatches the reset from its cleanup, so React's cleanup-before-effect guarantee means every seed — including the next level's after an in-place switch — lands on a fresh slice. The legacy "constants never change" invariant stays enforced between seed cycles; a test proves a changed channel without a reset still throws.This shape is only safe now that
useSources(#73868, merged into this branch) owns the channel: the inner view unmounts while the next level's project loads and the channel arrives as a fresh prop on remount, so nothing can re-seed a stale channel between the reset and the seed.Why not reset on
LifecycleEvent.LevelChangeRequestedTried first — it's the formal seam, and the event does fire. But the seed effect re-runs when
progress.currentLevelIdchanges, while the old view is still briefly mounted with the old channel: it re-seeds the old value right over the event-ordered reset, and the throw comes back when the new level seeds. Tying the reset to the seed effect's own cleanup is immune to this: reset and seed are paired by React's effect ordering rather than racing through event timing.(An earlier revision of this PR used a catch-and-retry wrapper — delegate to the legacy reducer, re-apply from initial state when its change-guard throws — because before #73868 the stale-channel race made any ordered reset unwinnable. With the race gone, the explicit reset replaces the exception-as-control-flow.)
Testing