Automigration: Move RN on-device addons to deviceAddons#34659
Merged
Conversation
…dons` Adds a new `rn-ondevice-addons-to-device-addons` automigration that moves on-device addons (packages whose name contains `ondevice`) from the shared `addons` array into a new `deviceAddons` array in `.rnstorybook/main.ts`. This is needed because Storybook Core evaluates every entry in `addons` as a Node.js preset, which on-device addons are not, and that causes `storybook extract` to fail. Split out of #34333 (M8). Tracking issue: #34276. Made-with: Cursor
ndelangen
added
maintenance
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new automigration fix for React Native on-device configurations. The change includes documentation explaining the migration of Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
code/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.test.ts (1)
25-46: ⚡ Quick winAlign Vitest mocks with repository spy-mocking rules.
These module mocks use factory-only
vi.mock(...)patterns. The repo guideline requires spy-based mocks and behavior setup throughbeforeEachfor consistency and typing safety.As per coding guidelines, “Use
vi.mock()with thespy: trueoption for all package and file mocks in Vitest tests” and “Implement mock behaviors inbeforeEachblocks in Vitest tests.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.test.ts` around lines 25 - 46, Replace the factory-only mocks with spy-based mocks and move behavior setup into beforeEach: call vi.mock('node:fs', { spy: true }) and in beforeEach set a spy implementation for existsSync to use mocks.existsSyncOverride or the real existsSync; vi.mock('../helpers/mainConfigFile', { spy: true }) and in beforeEach set updateMainConfig to mocks.updateMainConfig; and vi.mock('storybook/internal/common', { spy: true }) and in beforeEach set spies for findConfigFile and loadMainConfig to call the original implementations or test doubles as needed. Target the mocked symbols existsSync, updateMainConfig, findConfigFile, and loadMainConfig and ensure all behavioral wiring lives inside beforeEach for typing and repo consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@code/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.test.ts`:
- Around line 58-59: The mocks for findConfigFile and loadMainConfig are
implemented by returning the mocked symbols themselves, which creates
self-referential recursion; update the test to provide the real implementations
instead (e.g., capture the unmocked implementations via
vi.importActual/vi.requireActual or store originals before mocking) and use
those in vi.mocked(...).mockImplementation(...) for findConfigFile and
loadMainConfig so fallback branches call the real functions rather than the
mocks (refer to the mocked symbols findConfigFile and loadMainConfig in the
test).
In
`@code/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.ts`:
- Around line 130-139: The loop in run iterates all result.targets including
ones where ondeviceAddons is an empty array, causing unnecessary calls to
updateMainConfig; modify run to skip targets with no ondeviceAddons by checking
each target's ondeviceAddons length (e.g., if (!ondeviceAddons ||
ondeviceAddons.length === 0) continue) before calling updateMainConfig so only
targets with actual addons are processed; updateMainConfig, run, result.targets
and ondeviceAddons are the symbols to locate and change.
---
Nitpick comments:
In
`@code/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.test.ts`:
- Around line 25-46: Replace the factory-only mocks with spy-based mocks and
move behavior setup into beforeEach: call vi.mock('node:fs', { spy: true }) and
in beforeEach set a spy implementation for existsSync to use
mocks.existsSyncOverride or the real existsSync;
vi.mock('../helpers/mainConfigFile', { spy: true }) and in beforeEach set
updateMainConfig to mocks.updateMainConfig; and
vi.mock('storybook/internal/common', { spy: true }) and in beforeEach set spies
for findConfigFile and loadMainConfig to call the original implementations or
test doubles as needed. Target the mocked symbols existsSync, updateMainConfig,
findConfigFile, and loadMainConfig and ensure all behavioral wiring lives inside
beforeEach for typing and repo consistency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f0088d71-d90f-4bb5-8fd7-51936e97c793
📒 Files selected for processing (4)
MIGRATION.mdcode/lib/cli-storybook/src/automigrate/fixes/index.tscode/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.test.tscode/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.ts
20 tasks
Updated the migration documentation for React Native to specify that on-device addons must now be listed under the `deviceAddons` key in `.rnstorybook/main.ts`. Enhanced the automigration logic to ensure it correctly identifies and renames the `addons` key to `deviceAddons`, while also improving test coverage for various scenarios related to the migration process.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
code/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.test.ts (1)
29-53: 💤 Low valueAdd
spy: trueoption tovi.mock()calls.As per coding guidelines,
vi.mock()should use thespy: trueoption for all package and file mocks. This applies to the mocks fornode:fs,../helpers/mainConfigFile, andstorybook/internal/common.♻️ Example fix for one mock
-vi.mock('storybook/internal/common', async (importOriginal) => { +vi.mock('storybook/internal/common', { spy: true }, async (importOriginal) => { const mod = await importOriginal<typeof import('storybook/internal/common')>(); return { ...mod, findConfigFile: vi.fn(() => undefined), loadMainConfig: vi.fn(), }; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.test.ts` around lines 29 - 53, The vi.mock calls in this test file lack the required spy option; update each vi.mock invocation for 'node:fs', '../helpers/mainConfigFile', and 'storybook/internal/common' to pass { spy: true } as the second argument so the original module is spied on; leave the async factory functions and returned overrides (the existsSync override that uses mocks.existsSyncOverride, the updateMainConfig override, and the findConfigFile/loadMainConfig spies) intact but add the { spy: true } option to each vi.mock call signature.code/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.ts (1)
10-10: 💤 Low valueConsider using a package import instead of a cross-package relative path.
This deep relative path (
../../../../../core/src/...) crosses fromcli-storybookintocore's internal structure. Ifcorerestructures, this import breaks. IfRN_STORYBOOK_DIRis exported viastorybook/internal/commonor a similar package path, prefer that for consistency with the other imports in this file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.ts` at line 10, The import uses a fragile cross-package relative path for RN_STORYBOOK_DIR; replace that relative import with the stable package export (e.g. import RN_STORYBOOK_DIR from 'storybook/internal/common' or whatever public module in core exports it) so the module no longer depends on core's internal folder layout; update the import statement in rn-ondevice-addons-to-device-addons.ts to reference the package-level export that re-exports RN_STORYBOOK_DIR.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@code/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.test.ts`:
- Around line 29-53: The vi.mock calls in this test file lack the required spy
option; update each vi.mock invocation for 'node:fs',
'../helpers/mainConfigFile', and 'storybook/internal/common' to pass { spy: true
} as the second argument so the original module is spied on; leave the async
factory functions and returned overrides (the existsSync override that uses
mocks.existsSyncOverride, the updateMainConfig override, and the
findConfigFile/loadMainConfig spies) intact but add the { spy: true } option to
each vi.mock call signature.
In
`@code/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.ts`:
- Line 10: The import uses a fragile cross-package relative path for
RN_STORYBOOK_DIR; replace that relative import with the stable package export
(e.g. import RN_STORYBOOK_DIR from 'storybook/internal/common' or whatever
public module in core exports it) so the module no longer depends on core's
internal folder layout; update the import statement in
rn-ondevice-addons-to-device-addons.ts to reference the package-level export
that re-exports RN_STORYBOOK_DIR.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2520fe56-e610-4cc6-b0cf-8ad5038c3391
📒 Files selected for processing (3)
MIGRATION.mdcode/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.test.tscode/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.ts
✅ Files skipped from review due to trivial changes (1)
- MIGRATION.md
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new CLI automigration to fix React Native on-device Storybook configs by renaming the addons field in .rnstorybook/main.* to deviceAddons, preventing RN on-device addons from being evaluated as Node.js presets during tasks like storybook extract.
Changes:
- Introduces
rn-ondevice-addons-to-device-addonsautomigration that targets RN mains and renamesaddons→deviceAddonsvia AST transforms. - Adds unit tests covering RN vs web main detection and the key-rename behavior.
- Updates
MIGRATION.mdwith a new 10.3 → 10.4 entry documenting the change and automigration.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| code/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.ts | Implements the automigration: detects RN mains and renames addons → deviceAddons. |
| code/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.test.ts | Adds unit tests for selection logic (RN vs RNW) and AST rename behavior. |
| code/lib/cli-storybook/src/automigrate/fixes/index.ts | Registers the new fix in the allFixes list. |
| MIGRATION.md | Documents the migration and provides before/after examples for RN configs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Agent-Logs-Url: https://github.com/storybookjs/storybook/sessions/838b61dd-d549-417a-b74d-a0bb2f49961d Co-authored-by: ndelangen <3070389+ndelangen@users.noreply.github.com>
Agent-Logs-Url: https://github.com/storybookjs/storybook/sessions/1cf492bb-b5ad-414e-9e0f-5453da78c35d Co-authored-by: ndelangen <3070389+ndelangen@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@code/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.ts`:
- Around line 53-63: Replace the value-based idempotency check in
hasAddonsToRename so it detects the presence of the deviceAddons property
regardless of its value by using Object.prototype.hasOwnProperty.call(cfg,
'deviceAddons') instead of (cfg as { deviceAddons?: unknown }).deviceAddons !==
undefined; and add a defensive presence check at the start of the run() function
(the function that mutates cfg to set deviceAddons) to skip mutation (or bail)
if Object.prototype.hasOwnProperty.call(cfg, 'deviceAddons') is true to avoid
clobbering an existing field.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 27c5711d-c7bc-4cb1-8161-5b26b550b18b
📒 Files selected for processing (3)
code/core/src/manager/globals/exports.tscode/lib/cli-storybook/src/automigrate/fixes/rn-ondevice-addons-to-device-addons.tsdocs/configure/integration/eslint-plugin.mdx
💤 Files with no reviewable changes (1)
- code/core/src/manager/globals/exports.ts
✅ Files skipped from review due to trivial changes (1)
- docs/configure/integration/eslint-plugin.mdx
6 tasks
This change modifies the mock implementation of `findConfigFile` in the `rn-ondevice-addons-to-device-addons` test to return `null` instead of `undefined`, ensuring more accurate test behavior.
This change updates the type casting for the `node` variable in the `rn-ondevice-addons-to-device-addons` migration logic, ensuring compatibility when setting the `deviceAddons` field in the main configuration.
JReinhold
reviewed
May 1, 2026
JReinhold
approved these changes
May 1, 2026
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.
Split out of #34333 (M8).
Tracking issue: #34276.
What I did
Adds a new
rn-ondevice-addons-to-device-addonsautomigration for React Native projects that renames theaddonskey in.rnstorybook/main.tstodeviceAddons.Storybook Core evaluates every entry in
addonsas a Node.js preset. On-device addons (@storybook/addon-ondevice-*) are not Node.js presets, and listing them underaddonscausesstorybook extractto fail.Change of heart
The first iteration of this codemod tried to be clever: it scanned the
addonsarray, detected on-device entries by matching the substringondevicein their package name, and moved only those entries into a newdeviceAddonsarray, leaving everything else underaddons.After discussing it with @shilman, we decided that the approach was both too narrow and too presumptuous:
.rnstorybook/main.tshas no use for the regularaddonskey in the first place: RN does not run a Storybook builder, so nothing underaddonswould ever be evaluated as a Node.js preset usefully. In practice, anything you list there is a "device addon".on-devicesubstring heuristic would silently miss any community/third-party on-device addon that doesn't follow that naming convention.So the new behavior is simpler: rename the
addonskey todeviceAddons. The whole array moves verbatim — strings, object-form entries, comments (best-effort, as with all our AST transforms) — and no entry is dropped or reclassified.How it works now
@storybook/react-nativeis not in the project's dependencies.main.tsfiles: the active config plus a sibling when the project has both.storybook/and.rnstorybook/(the React-Native + RNW setup)..rnstorybook(theRN_STORYBOOK_DIRconstant), ORframeworkfield resolves to@storybook/react-native.Anything else — notably
@storybook/react-native-web-viteor any other web framework — is treated as a web main and skipped.addonsfield is a non-empty array ANDdeviceAddonsis not already present (idempotency: re-running the migration is a no-op).For every selected target, perform an AST-level key rename:
This means in a paired RN + RNW setup, only
.rnstorybook/main.tsis migrated; the web.storybook/main.tsis left completely untouched.MIGRATION.mdhas been rewritten to describe the simpler "rename the whole key" behavior, with a matching before/after example.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Manually ran the migration on:
addonsarray containing both@storybook/addon-ondevice-controlsand@storybook/addon-docs— both moved underdeviceAddons, original key removed..storybook/(web,@storybook/react-native-web-vite) +.rnstorybook/setup — only.rnstorybook/main.tswas modified.deviceAddonswas already defined — no-op, as expected.Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add `ci:normal`, `ci:merged` or `ci:daily` GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in `code/lib/cli-storybook/src/sandbox-templates.ts`
Make sure this PR contains one of the labels below:
Available labels
🦋 Canary release
This pull request has been released as version
0.0.0-pr-34659-sha-a82cc511. Try it out in a new sandbox by runningnpx storybook@0.0.0-pr-34659-sha-a82cc511 sandboxor in an existing project withnpx storybook@0.0.0-pr-34659-sha-a82cc511 upgrade.More information
0.0.0-pr-34659-sha-a82cc511norbert/m8-device-addons-automigrationa82cc5111777542403)To request a new release of this pull request, mention the
@storybookjs/coreteam.core team members can create a new canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=34659Summary by CodeRabbit
New Features
addonstodeviceAddonsin React Native on-device main configs.Documentation