Use loadStory to switch stories on a single surface instead of remounting#92
Draft
EmilioBejasa wants to merge 12 commits intomainfrom
Draft
Use loadStory to switch stories on a single surface instead of remounting#92EmilioBejasa wants to merge 12 commits intomainfrom
EmilioBejasa wants to merge 12 commits intomainfrom
Conversation
…ting Previously a new ReactSurface was created and destroyed for each story, forcing createPreparedStoryMapping() to re-run every time. On a slow CI emulator this caused awaitStoryReady() to time out, producing screenshots of the "Loading story..." state. Now a single surface is kept alive for all stories. The first story is passed as the initial prop; subsequent stories are switched by emitting the loadStory native event, which updates activeStoryName state in StoryRenderer and triggers a re-render. _idToPrepared is built once on the first render and reused for all subsequent switches. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Closed
NativeEventEmitter(module) requires the native module to implement addListener/removeListeners — StorybookRegistry doesn't, so events were silently dropped. The native side emits via DeviceEventManagerModule, so the correct JS receiver is DeviceEventEmitter. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…chris-workspace into fix/use-load-story-for-screenshots
notifyStoryReady() fires in JS useEffect after React's commit, but
Fabric dispatches view-tree mutations to the main thread asynchronously.
Without a sync point, the test thread could screenshot the stale (first
story) view before the main thread has applied the new story's mutations.
runOnMainSync {} flushes all pending main-thread tasks — including
Fabric's mutations — before Screenshot.snap().record() draws the view.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ming
runOnMainSync{} only drains the Handler queue. Fabric dispatches mount
items via postFrameCallback (Choreographer), which is separate from the
Handler — so runOnMainSync can return before Fabric has applied the new
story's mutations.
Posting our own postFrameCallback ensures Fabric's earlier-registered
callback (queued during the React commit, before useEffect fired) always
runs first in the same vsync. The native view hierarchy is guaranteed to
reflect the current story before Screenshot.snap().record() draws it.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The library detects when it's already on the UI thread and skips its own Handler dispatch, so there's no deadlock. Being explicit here makes it clear that the draw always happens on the main thread, consistent with the Choreographer frame wait above. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
DeviceEventEmitter emission from native is unreliable in bridgeless new-arch mode (RN 0.82 + newArchEnabled=true), causing awaitStoryReady to time out and every screenshot to show the first story. Fix by mounting a fresh surface for each story with the correct storyName prop, removing the need for loadStory events entirely. Also adds try/finally to renderStory so surface cleanup always runs even if the lambda throws. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
storyNameToId() only split on the first "/" so stories with hierarchical titles like "Example/Button" produced wrong IDs (e.g. "example--button" instead of "example-button--primary"), causing those stories to fail with "story not found". Fix by passing story.id (already the correct Storybook ID from the manifest) directly as the storyName prop and using it verbatim in StoryRenderer, removing the lossy title/name round-trip entirely. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Bootstrap was tearing down as soon as the manifest file appeared, but createPreparedStoryMapping() was still running async. Stories starting right after bootstrap found _idToPrepared empty and had to wait for it themselves, non-deterministically exceeding the 5s timeout. Fix: prepareForNextStory() before bootstrap so awaitStoryReady() can block until JS calls notifyStoryReady() — which only fires after createPreparedStoryMapping() completes. _idToPrepared is then fully populated before any story surface starts. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The manifest persists on device between test runs, so the conditional bootstrap was skipped on re-runs and _idToPrepared was empty when the story loop started. Stories then had to await createPreparedStoryMapping themselves, non-deterministically hitting the 5s timeout. Fix: unconditionally run bootstrapManifest before the story loop so _idToPrepared is always populated and the manifest is always fresh. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Instead of remounting a fresh ReactSurface for every story, keep one surface alive for the entire test run. After the bootstrap render populates the story manifest and _idToPrepared, each story is switched by emitting a loadStory event from the main thread and waiting on a CountDownLatch that notifyStoryReady() releases. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
getJSModule(RCTDeviceEventEmitter) is a bridge-era API that returns null in bridgeless/new-arch mode, silently dropping the event. emitDeviceEvent is the current API used by RN's own DeviceEventManagerModule internals. Co-Authored-By: Claude Sonnet 4.6 <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
1 participant
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
ReactSurfacewas created and destroyed for each story, forcingcreatePreparedStoryMapping()to re-run every time. On a slow CI emulator this causedawaitStoryReady()to time out, producing screenshots of the "Loading story..." state.loadStorynative event (from PR Add loadStory event emitter (native→JS) #74), which updatesactiveStoryNamestate inStoryRendererand triggers a re-render._idToPreparedis built once and reused.Changes
loadStoryevent now drives re-renders viaactiveStoryNamestate instead of just loggingloadStory()via companion object so the test can call itloadStory()to switchTest plan