feat:unified withStorybook wrapper for entrypoint-swapping#871
feat:unified withStorybook wrapper for entrypoint-swapping#871
withStorybook wrapper for entrypoint-swapping#871Conversation
🦋 Changeset detectedLatest commit: 5c4dc2b The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@copilot, can you try again? It looks like you had some internal error. |
…overrides to withStorybook M4 implementation: - Add resolveEntryPoint() to detect app entry (Expo, Expo Router, RN CLI) - Add entry-point swapping in resolver when STORYBOOK_ENABLED=true - Add websocket env variable overrides (STORYBOOK_WS_HOST/PORT/SECURED) - Add comprehensive tests for all new functionality Agent-Logs-Url: https://github.com/storybookjs/react-native/sessions/58a5fb17-1ee3-4e0b-ae60-aee474de2d32 Co-authored-by: ndelangen <3070389+ndelangen@users.noreply.github.com>
Agent-Logs-Url: https://github.com/storybookjs/react-native/sessions/58a5fb17-1ee3-4e0b-ae60-aee474de2d32 Co-authored-by: ndelangen <3070389+ndelangen@users.noreply.github.com>
Implemented M4 in commits a889fb4 and 537de98. The changes add:
|
|
@copilot Did you catch that we'd need backwards support? I think it'd be best if we created 2 separate |
- Restore withStorybook to original behavior (no entry-point swapping, no ws env overrides) - Create withStorybookSwap with entry-point swapping + ws env overrides - Export shared utilities (resolveEntryPoint, resolveFileWithExtensions, resolveStorybookEntry, applyWebsocketEnvOverrides) from withStorybook.ts - Add metro/withStorybookSwap.js re-export and package.json export path - Add withStorybookSwap to tsup build entries - Tests: 33 passing across 5 suites Agent-Logs-Url: https://github.com/storybookjs/react-native/sessions/a3213ee7-9a44-4b9a-a4b2-f434af51c3ce Co-authored-by: ndelangen <3070389+ndelangen@users.noreply.github.com>
Agent-Logs-Url: https://github.com/storybookjs/react-native/sessions/a3213ee7-9a44-4b9a-a4b2-f434af51c3ce Co-authored-by: ndelangen <3070389+ndelangen@users.noreply.github.com>
|
@copilot I'm aiming for the new config wrapper to be a single agnostic one, exposed as This exports a single named export ( The types of this function should be like so: Though internally we should try to be somewhat type-safe, but externally it's not exposed. |
…/withStorybook Creates a bundler-agnostic withStorybook<T>(config: T, options?): T that auto-detects Metro vs Repack and delegates accordingly: - Metro: entry-point swapping + ws env overrides (via withStorybookSwap) - Repack: adds StorybookPlugin with ws env overrides to plugins array This allows sb init to always generate the same config regardless of bundler. Agent-Logs-Url: https://github.com/storybookjs/react-native/sessions/e08bd14d-760c-471e-938c-e26d89e9ae9b Co-authored-by: ndelangen <3070389+ndelangen@users.noreply.github.com>
Agent-Logs-Url: https://github.com/storybookjs/react-native/sessions/e08bd14d-760c-471e-938c-e26d89e9ae9b Co-authored-by: ndelangen <3070389+ndelangen@users.noreply.github.com>
Added a unified
Usage is identical regardless of bundler: const { withStorybook } = require('@storybook/react-native/withStorybook');
module.exports = withStorybook(config);The existing |
|
@copilot the previously existing ./metro/withStorybook and ./repack/withStorybook ARE the backwards support, and should remain completely untouched. The |
…ore addons (#874) * feat: add deviceAddons property for on-device addons separation - Add `deviceAddons` to `StorybookConfig` type - Update `generate.js` to merge addons from both `addons` and `deviceAddons` - Update init template to use `deviceAddons` for on-device addons - Update expo-example to use `deviceAddons` - Add tests for deviceAddons and mixed addons scenarios - Backwards compatible: addons in `addons` field still work Agent-Logs-Url: https://github.com/storybookjs/react-native/sessions/82703242-efa9-4fae-8cca-c667593bb37f Co-authored-by: ndelangen <3070389+ndelangen@users.noreply.github.com> * chore: add changeset for deviceAddons feature Agent-Logs-Url: https://github.com/storybookjs/react-native/sessions/b99f73e3-2330-42e2-8149-ab13e5f3892c Co-authored-by: ndelangen <3070389+ndelangen@users.noreply.github.com> * fix: update deviceAddons test snapshots after base branch merge Agent-Logs-Url: https://github.com/storybookjs/react-native/sessions/0102d455-cd0f-430c-bab9-f3921b3ad9b8 Co-authored-by: ndelangen <3070389+ndelangen@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: ndelangen <3070389+ndelangen@users.noreply.github.com> Co-authored-by: Norbert de Langen <ndelangen@me.com>
…thStorybook - Enhanced logging in the View component to provide clearer output when a persisted story is not found. - Updated the handling of the liteMode option in withStorybook to use a new environment variable for better configuration management. All tests pass successfully.
…om:storybookjs/react-native into copilot/add-entry-point-swapping-storybook
|
@dannyhw I've verified this work for existing setups and is backwards compatible |
|
Will take a look soon, thanks |
… Storybook - Introduced a minimal WebSocket server for manual connectivity checks, configurable via environment variables `STORYBOOK_WS_HOST`, `STORYBOOK_WS_PORT`, and `STORYBOOK_WS_SECURED`. - Updated `withStorybook` and `withStorybook.test.ts` to utilize these environment variables for improved configuration management. - Enhanced the handling of WebSocket options to allow for dynamic binding based on environment settings. All tests pass successfully.
There was a problem hiding this comment.
Pull request overview
Adds a new unified, bundler-agnostic withStorybook wrapper to @storybook/react-native, introduces a deviceAddons config field, and updates the generator/tests to support the new addon split and lite-mode plumbing.
Changes:
- Add
@storybook/react-native/withStorybookand internal config enhancers for Metro/Repack entry swapping. - Add
deviceAddonstoStorybookConfigand update generator + fixtures/tests to import addons from bothaddonsanddeviceAddons. - Extend runtime/UI to support lite-mode behavior (options propagation, UI toggling) and add a manual WS smoke test script.
Reviewed changes
Copilot reviewed 33 out of 34 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Removes baseUrl setting. |
| tests/scripts/mocks/mixed-addons/preview.js | Adds mock preview with decorators/parameters for mixed addons. |
| tests/scripts/mocks/mixed-addons/main.js | Adds mock main config with addons + deviceAddons. |
| tests/scripts/mocks/mixed-addons/FakeStory.stories.tsx | Adds mock story file for generator tests. |
| tests/scripts/mocks/mixed-addons/FakeComponent.tsx | Adds mock component for generator tests. |
| tests/scripts/mocks/device-addons/preview.js | Adds mock preview for deviceAddons-only scenario. |
| tests/scripts/mocks/device-addons/main.js | Adds mock main config using deviceAddons. |
| tests/scripts/mocks/device-addons/FakeStory.stories.tsx | Adds mock story file for generator tests. |
| tests/scripts/mocks/device-addons/FakeComponent.tsx | Adds mock component for generator tests. |
| tests/scripts/generate.test.ts.snapshot | Updates snapshots for new generated output (options + addon import behavior). |
| tests/scripts/generate.test.ts | Adds tests covering deviceAddons and mixed addon lists. |
| packages/react-native/tsup.config.ts | Adds src/withStorybook.ts as a build entry. |
| packages/react-native/template/cli/main.ts | Updates template to use deviceAddons instead of addons. |
| packages/react-native/src/withStorybook.ts | Introduces new unified wrapper with shared setup + bundler detection. |
| packages/react-native/src/withStorybook.test.ts | Adds unit tests for unified wrapper behavior/env overrides. |
| packages/react-native/src/View.tsx | Threads options into View and changes lite-mode/UI behavior and story-only rendering wrapper. |
| packages/react-native/src/Start.tsx | Passes options into View constructor. |
| packages/react-native/src/repack/withStorybook.ts | Updates Repack plugin to apply STORYBOOK_WS_* env overrides and revised server/generate behavior. |
| packages/react-native/src/prepareStories.ts | Adds liteMode to ReactNativeOptions. |
| packages/react-native/src/metro/withStorybook.ts | Updates Metro wrapper to apply STORYBOOK_WS_* env overrides and revised server/generate behavior. |
| packages/react-native/src/metro/withStorybook.test.ts | Adds test for env-based websockets when option omitted. |
| packages/react-native/src/metro/utils.ts | Adds shared utilities (entry/storybook entry resolution) and shared options type. |
| packages/react-native/src/metro/utils.test.ts | Adds tests for entry-point resolution logic. |
| packages/react-native/src/index.ts | Makes addons optional and introduces deviceAddons in StorybookConfig. |
| packages/react-native/src/enhanceRepackConfig.ts | Adds internal helper to swap Repack entrypoint. |
| packages/react-native/src/enhanceRepackConfig.test.ts | Adds tests for Repack entry swapping behavior. |
| packages/react-native/src/enhanceMetroConfig.ts | Adds internal helper to adjust Metro config and swap entrypoint via resolver. |
| packages/react-native/src/enhanceMetroConfig.test.ts | Adds tests for Metro enhancements and entry swapping. |
| packages/react-native/scripts/ws-smoke-server.mjs | Adds a manual WS connectivity smoke server script. |
| packages/react-native/scripts/generate.js | Extends generator to merge addons + deviceAddons and propagate liteMode into options. |
| packages/react-native/package.json | Exports new ./withStorybook entry and adds ws-smoke-server script. |
| examples/expo-example/.rnstorybook/main.ts | Splits addons into addons + deviceAddons in example config. |
| .changeset/unified-withstorybook-wrapper.md | Changeset for the new unified wrapper export. |
| .changeset/add-device-addons.md | Changeset for introducing deviceAddons config field. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| configPath, | ||
| useJs, | ||
| docTools, | ||
| ...(!!host ? { host: host, port: websockets.port, secured: !websockets.secured } : {}), |
There was a problem hiding this comment.
withStorybook passes secured: !websockets.secured into generate(), which inverts the intended meaning and also turns secured on when websockets.secured is undefined (because !undefined === true). This will generate a wss:// URL when it should be ws:// (and vice-versa). Pass the actual secured value (defaulting to false) consistently with createChannelServer and the legacy wrappers.
| ...(!!host ? { host: host, port: websockets.port, secured: !websockets.secured } : {}), | |
| ...(!!host | |
| ? { host: host, port: websockets.port, secured: websockets.secured ?? false } | |
| : {}), |
There was a problem hiding this comment.
this seems like a valid point, just on face level
| if (server || experimental_mcp) { | ||
| createChannelServer({ | ||
| port: websockets.port, | ||
| host: websockets.host, | ||
| configPath, | ||
| experimental_mcp, | ||
| websockets: Boolean(websockets.host), | ||
| secured: websockets.secured, |
There was a problem hiding this comment.
WebSocket enablement is derived from Boolean(websockets.host), which disables websockets for valid configurations like websockets: 'auto' (host intentionally undefined) or websockets: { port: 7007 }. This makes the unified wrapper behave differently from src/metro/withStorybook.ts / src/repack/withStorybook.ts, where 'auto' still enables the WS server. Consider computing a websocketsEnabled flag from the presence of the websockets option or STORYBOOK_WS_HOST, and bind host separately.
| export function withStorybook<T>(config: T, options: WithStorybookOptions = {}): T { | ||
| const enabled = envVariableToBoolean(process.env.STORYBOOK_ENABLED, false); | ||
| if (!enabled) { | ||
| return config; | ||
| } | ||
| const server = envVariableToBoolean(process.env.STORYBOOK_SERVER, true); |
There was a problem hiding this comment.
The unified withStorybook accepts WithStorybookOptions (which includes enabled?: boolean), but the implementation ignores options.enabled and only checks process.env.STORYBOOK_ENABLED. This is an API footgun for consumers expecting parity with the existing Metro wrapper options. Either support options.enabled here, or define a dedicated options type for the unified wrapper that omits enabled.
| _options: any; | ||
| _idToPrepared: Record<string, PreparedStory<ReactRenderer>> = {}; | ||
|
|
||
| constructor(preview: PreviewWithSelection<ReactRenderer>, channel: Channel) { | ||
| constructor(preview: PreviewWithSelection<ReactRenderer>, channel: Channel, options: any) { | ||
| this._preview = preview; | ||
| this._channel = channel; | ||
| this._options = options; | ||
| } | ||
|
|
There was a problem hiding this comment.
start() passes options?: ReactNativeOptions through to new View(...), but options is optional and may be undefined. View.getStorybookUI() then unconditionally reads this._options.liteMode, which will throw if _options is undefined. Default options to {} either in start() or in the View constructor (and consider updating updateView() to refresh _options as well).
| const storage = params.storage ?? { | ||
| getItem: async (key) => null, | ||
| setItem: async (key, value) => {}, | ||
| }; | ||
|
|
||
| const onDeviceUI = this._options.liteMode ? false : (params.onDeviceUI ?? true); | ||
| const shouldPersistSelection = this._options.liteMode | ||
| ? false | ||
| : (params.shouldPersistSelection ?? true); | ||
|
|
There was a problem hiding this comment.
storage is now defaulted to a no-op implementation, which makes the later warning if (shouldPersistSelection && !storage) unreachable and can silently disable persistence (writes go to the no-op storage). If persistence requires a real storage implementation, keep storage undefined by default and warn, or change the warning condition to check params.storage (the user-provided value) while still allowing a safe fallback for non-persisting modes.
| const resolveResult = resolveFunction(theContext, moduleName, platform); | ||
|
|
||
| if (resolveResult?.filePath?.includes?.('@storybook/react/template/cli')) { | ||
| return { type: 'empty' }; | ||
| } | ||
|
|
||
| if (moduleName === 'tty' || moduleName === 'os') { | ||
| return { type: 'empty' }; | ||
| } | ||
|
|
There was a problem hiding this comment.
resolveRequest calls resolveFunction(...) before handling the known Node core-module shims (tty/os). If the resolver throws on these (the workaround exists elsewhere in the codebase), this handler won't get a chance to return { type: 'empty' }. Consider short-circuiting tty/os before calling resolveFunction (as done in src/metro/withStorybook.ts).
| const resolveResult = resolveFunction(theContext, moduleName, platform); | |
| if (resolveResult?.filePath?.includes?.('@storybook/react/template/cli')) { | |
| return { type: 'empty' }; | |
| } | |
| if (moduleName === 'tty' || moduleName === 'os') { | |
| return { type: 'empty' }; | |
| } | |
| if (moduleName === 'tty' || moduleName === 'os') { | |
| return { type: 'empty' }; | |
| } | |
| const resolveResult = resolveFunction(theContext, moduleName, platform); | |
| if (resolveResult?.filePath?.includes?.('@storybook/react/template/cli')) { | |
| return { type: 'empty' }; | |
| } |
| import { createServer } from 'node:http'; | ||
| import { WebSocketServer } from 'ws'; | ||
|
|
||
| const PING_INTERVAL_MS = 10_000; | ||
|
|
||
| const port = Number(process.env.STORYBOOK_WS_PORT) || 7007; | ||
| const host = process.env.STORYBOOK_WS_HOST || undefined; | ||
|
|
||
| const httpServer = createServer((_req, res) => { | ||
| res.writeHead(404); | ||
| res.end(); | ||
| }); | ||
|
|
||
| const wss = new WebSocketServer({ server: httpServer }); | ||
|
|
||
| // Same global ping interval as createChannelServer — keeps Storybook client transport alive. | ||
| const pingInterval = setInterval(() => { | ||
| wss.clients.forEach((client) => { | ||
| if (client.readyState === WebSocket.OPEN) { | ||
| client.send(JSON.stringify({ type: 'ping', args: [] })); | ||
| } |
There was a problem hiding this comment.
ws-smoke-server.mjs references WebSocket.OPEN but only imports WebSocketServer from ws. This will throw a ReferenceError at runtime. Import WebSocket (or compare against the numeric readyState value / client.OPEN) before using it.
| function envVariableToBoolean(value: string | undefined, defaultValue: any = false): boolean { | ||
| switch (value) { | ||
| case 'true': | ||
| return true; | ||
| case 'false': | ||
| return false; | ||
| default: | ||
| return !!defaultValue; |
There was a problem hiding this comment.
PR description states legacy ./metro/withStorybook and ./repack/withStorybook wrappers are "completely untouched", but this PR modifies both (e.g. adds env override handling and changes server/generate behavior). Please update the PR description to match the actual changes, or move these updates behind the new unified wrapper if the intent is to keep the legacy entrypoints unchanged.
- Reorganized scripts in package.json files to improve consistency and clarity, including the addition of `check:types` and reordering of existing scripts. - Updated dependencies and added missing fields such as `license` and `files` in several package.json files. - Enhanced the handling of `deviceAddons` in the configuration files to ensure proper migration warnings for legacy usage. - Added new test cases to validate the migration of on-device addons and ensure backward compatibility. All tests pass successfully.
ndelangen
changed the title
withStorybook wrapper for entrypoint-swapping
- Updated the `StorybookConfig` interface to mark the `addons` field as deprecated, advising users to migrate to `deviceAddons`. - Enhanced the `generate.js` script to log a deprecation warning when `addons` is used, guiding users to move their entries to `deviceAddons`. - Adjusted tests to verify the deprecation warning functionality for non-empty `addons`. All tests pass successfully.
…mment) - Updated the condition to disable experimental MCP settings when the server is not present, ensuring proper configuration behavior. - This change enhances the reliability of the withStorybook function in various environments. All tests pass successfully.
- Adjusted the formatting of the `names` variable declaration for improved clarity. - This change enhances the overall readability of the script without altering its functionality. All tests pass successfully.
| docTools, | ||
| ...(!!host ? { host: host, port: websockets.port, secured: !websockets.secured } : {}), | ||
| liteMode, | ||
| } as any); |
There was a problem hiding this comment.
would be cool if we could avoid this type cast
| @@ -0,0 +1,56 @@ | |||
| /** | |||
| * Minimal HTTP + WebSocket server for manual connectivity checks (LAN, firewall). | |||
There was a problem hiding this comment.
I'm not sure I understand what this smoke server is about
| "serve": "docusaurus serve", | ||
| "write-translations": "docusaurus write-translations", | ||
| "write-heading-ids": "docusaurus write-heading-ids", | ||
| "start": "docusaurus start", |
There was a problem hiding this comment.
what changed here? just the ordering?
| * `storybook.requires`. Not evaluated as presets by Storybook Core, which | ||
| * avoids failures during server-side operations like `extract`. | ||
| */ | ||
| deviceAddons?: Addon[]; |
There was a problem hiding this comment.
would be curious to see if we added deviceAddons to the config type of react-native-web-vite if we could then work towards a single config folder
There was a problem hiding this comment.
also I guess it seems slightly odd if deviceAddons is needed in a react-native storybook specific config file. Like maybe the chromatic tooling could even handle the known edgecases of react native since they are small?
in v11 anyway i want to bring most of the ondevice addons into the core of rn storybook so maybe it gets less important then 🤔
| * Note that this is for future and play functions are not yet fully supported on native. | ||
| */ | ||
| playFn?: boolean; | ||
| liteMode?: boolean; |
There was a problem hiding this comment.
I was kinda thinking of moving this type of options to the features option and move to remove this ReactNativeOptions later, but open to thoughts on that
| import { patchChannelForRN } from './patchChannelForRN'; | ||
| import deepmerge from 'deepmerge'; | ||
| import { useEffect, useMemo, useReducer, useState } from 'react'; | ||
| import { StatusBar } from 'react-native'; |
There was a problem hiding this comment.
theres already a react-native import below which is causing this lint warning
| const exists = value && this._storyIdExists(value); | ||
|
|
||
| if (!exists) console.log('Storybook: could not find persisted story'); | ||
| if (!exists) { |
There was a problem hiding this comment.
nitpick: I don't think we need these formatting changes
| self._channel.emit(SET_CURRENT_STORY, { storyId: newStoryId }) | ||
| } | ||
| storage={storage} | ||
| storage={this._storage} |
There was a problem hiding this comment.
we have a linting warning here, I'd like to keep linting fully clean if possible
There was a problem hiding this comment.
arguably just the storage variable is fine here as it was
| } else { | ||
| return ( | ||
| <StoryView useWrapper={storyViewWrapper} storyBackgroundColor={storyBackgroundColor} /> | ||
| <SafeAreaProvider> |
There was a problem hiding this comment.
not sure I understand this part of the change, the ondeviceui is off in this scenario onDeviceUI==false so should we be adding any ui here?
is this intended specifically for the visual testing scenario?
sometimes the user would want no safearea here because they have a screen with a background color that fills the full area even on the safe area
we have a no safearea parameter maybe it could be a compromise to support that here
|
just left some initial thoughts so far, still need to test it out myself |
Tracked by storybookjs/storybook#34276 ("Easy React Native Setup")
Companion PR: storybookjs/storybook#34333 (CLI init overhaul, codemods, automigrations)
Closes #873 (
deviceAddons)Documentation PR: #877
What this does
This PR adds a bundler-agnostic
withStorybookwrapper at@storybook/react-native/withStorybookthat introduces entry-point swapping as the new recommended way to run Storybook for React Native — matching the mental model of Storybook for Web, where Storybook runs as its own entry point rather than being deeply integrated into the user's app.It also introduces the
deviceAddonsconfig property, separating on-device addons from theaddonsarray to preventCriticalPresetLoadErrorduring server-side operations likeextract.Entry-point swapping
The new
withStorybookwrapper:.rnstorybook/indexwhenSTORYBOOK_ENABLED=trueis set — no changes toApp.tsxrequiredSTORYBOOK_ENABLEDis not set — zero Storybook code in the production bundleSTORYBOOK_ENABLED,STORYBOOK_WS_HOST,STORYBOOK_WS_PORT,STORYBOOK_WS_SECURED,STORYBOOK_SERVER,STORYBOOK_DISABLE_UI), so the same metro config works for all environmentsstorybook.requiresfile, eliminating the need to manually match config betweenwithStorybookandgetStorybookUIThe existing
./metro/withStorybookand./repack/withStorybookexports are completely untouched and remain fully supported for projects using the in-app integration approach.deviceAddonsOn-device addons (like
@storybook/addon-ondevice-controls) contain React Native code that can't be evaluated in Node.js. When listed inaddons, Storybook Core tries to load them as presets during operations likeextract, which fails. The newdeviceAddonsproperty ensures they're only loaded at runtime on the device.Backwards compatible —
addonsstill works and a console warning guides users to migrate. The companion PR (storybookjs/storybook#34333) includes an automigration (rn-ondevice-addons-to-device-addons) that handles this automatically vianpx storybook automigrate.How it works together
This PR (react-native repo) provides the runtime bundler wrapper. The companion PR (storybook repo) provides the CLI tooling:
withStorybookdeviceAddonstype + runtimeStorybookConfigtype,generate.jsstorybook initfor RNdeviceAddonsautomigrationrn-ondevice-addons-to-device-addonswithStorybook()wrappingManual verification
Existing
withStorybook(backwards compatibility)Upgraded two existing projects to the canary release of this branch to verify the legacy
./metro/withStorybookpath is unaffected:Both projects continued to function correctly with their existing metro config and in-app Storybook integration, confirming zero regressions.
storybook initCLITested
npx storybook@0.0.0-pr-34333-sha-865b50bc initin a fresh Expo project:.rnstorybook/index.tsgenerated correctlymetro.config.jswrapped withwithStorybook()using the bundler-agnostic importstorybook:iosandstorybook:androidscripts added topackage.jsonmain.ts(and other templates) from thelatestpublished version of@storybook/react-native, not from the canary. This means the generatedmain.tswon't havedeviceAddonsuntil this PR is released. This will resolve itself once we publish the minor release.npx storybook automigrateVerified that
npx storybook@0.0.0-pr-34333-sha-865b50bc automigratecorrectly moves on-device addons fromaddons→deviceAddonsin an existing project's.rnstorybook/main.ts.Release plan
Targeting release as a new minor version this week (v10.4), alongside:
Test coverage
@storybook/react-native(unit tests forwithStorybook,enhanceMetroConfig,enhanceRepackConfig, entry-point resolution, env var handling)testspackage (story generation snapshots)