Docs: Update tsconfig.json diffs in addons migration guide#32922
Conversation
Dschungelabenteuer
requested review from
jonniebigodes and
kylegach
as code owners
Dschungelabenteuer
changed the title
tsconfig.json diffstsconfig.json diffs in addons migration guide
Dschungelabenteuer
changed the title
tsconfig.json diffs in addons migration guidetsconfig.json diffs in addons migration guide
|
View your CI Pipeline Execution ↗ for commit 5b21b1c
☁️ Nx Cloud last updated this comment at |
Dschungelabenteuer
added
ci:docs
📝 WalkthroughWalkthroughDocumentation updated to reflect ESM-first addon migration: package.json exports reworked to ES module defaults including "./manager-helpers"; bundler config moves from exportEntries to managerEntries/previewEntries; presets and local addon loading converted to ESM (import.meta.resolve); TS/tsup build settings adjusted; CSF Factories/preview wrapping added. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Main as main.ts
participant Resolver as import.meta.resolve
participant LocalPreset as .storybook/local-preset.ts
participant Build as Bundler (tsup)
participant Runtime as Storybook (manager/preview)
rect rgba(50,150,250,0.06)
Main->>Resolver: resolve local preset path
Resolver-->>Main: file URL
Main->>LocalPreset: dynamic import (ESM)
LocalPreset-->>Main: exports (managerEntries, previewEntries, manager-helpers)
end
rect rgba(50,200,100,0.06)
Main->>Build: provide managerEntries & previewEntries (ESM outputs, externals)
Build-->>Runtime: emit ES module artifacts (manager, preview, manager-helpers)
Runtime->>Runtime: load ES module outputs at runtime
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)docs/**📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧠 Learnings (4)📓 Common learnings📚 Learning: 2025-09-24T09:39:39.233ZApplied to files:
📚 Learning: 2025-10-13T13:33:14.659ZApplied to files:
📚 Learning: 2025-10-13T13:33:14.659ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/addons/addon-migration-guide.mdx (3)
118-131: Explain thesplittingbehavior change and its implications.Line 120 changes
splitting: falsetosplitting: true, which is a significant shift in bundler behavior. Splitting typically creates multiple output chunks rather than a single bundle, which can affect:
- Bundle size per entry point
- Loading patterns in the Storybook UI
- Tree-shaking effectiveness
This change appears intentional and beneficial for ESM-only builds, but the documentation should clarify why splitting was enabled and what addon developers should expect. Consider adding a brief note explaining the performance or architectural rationale.
59-67: Clarify the relationship between the bundler config changes and build outputs.The text on lines 62–67 summarizes key changes but could benefit from a brief note about the practical outcome: addon developers no longer need to maintain separate CJS and ESM bundles, reducing complexity. Additionally, the note on line 67 about switching from
exportEntriestopreviewEntries/managerEntriesis important, but the section title and intro (lines 58–60) could explicitly mention that this reconfigures where addon code gets bundled (browser vs. Node context), not just the build system.
304-333: Ensure themanager-helpersexport example is consistent with the broader bundler changes.The optional section on removal of
exportEntries(lines 304–333) provides a helpful pattern for exporting manager-specific code via a newsrc/manager-helpers.tsfile. This is well-explained, but consider adding a cross-reference or note that this approach mirrors the pattern shown earlier in the tsup.config.ts diff (lines 152–165), where manager entries are compiled separately withplatform: "browser".
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/addons/addon-migration-guide.mdx(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
docs/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Update documentation under
docs/for significant changes, including migration guides for breaking changes
Files:
docs/addons/addon-migration-guide.mdx
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to docs/** : Update documentation under `docs/` for significant changes, including migration guides for breaking changes
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to docs/** : Update documentation under `docs/` for significant changes, including migration guides for breaking changes
Applied to files:
docs/addons/addon-migration-guide.mdx
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
docs/addons/addon-migration-guide.mdx
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.{ts,tsx,js,jsx,mjs} : Export functions from modules when they need to be unit-tested
Applied to files:
docs/addons/addon-migration-guide.mdx
📚 Learning: 2025-10-13T13:33:14.659Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/tsconfig*.json : Keep TypeScript strict mode enabled in tsconfig (e.g., `"strict": true`)
Applied to files:
docs/addons/addon-migration-guide.mdx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Danger JS
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (3)
docs/addons/addon-migration-guide.mdx (3)
225-246: Verify TypeScript configuration values and document implications ofrootDirchange.The tsconfig.json diff shows several changes that appear correct for ESM:
"moduleResolution": "bundler"and"module": "Preserve"correctly preserve ES module semantics"target": "esnext"and"lib": ["esnext", ...]align with ESM re-bundling by downstream toolsHowever, a few points warrant verification:
Capitalization:
"module": "Preserve"uses capitalP. Verify this matches the exact TypeScript compiler option (it should be"Preserve", not"preserve").
rootDirimplication: Changing from"./src"to"."affects TypeScript's understanding of the project structure. Sincetsup.config.tsis now included in the TypeScript compilation via the include array, ensure this doesn't inadvertently pull in unwanted files (e.g.,node_modules, dist output, test files). Consider whether you need to add an explicit"exclude"array to the tsconfig for safety.
362-366: Verify the addon-kit PR reference and ensure it remains valid as the guide evolves.Line 364 references the addon-kit migration PR (#82) as a complete example. While this is helpful, consider:
- Confirming the PR link is stable and accessible.
- Adding a note (e.g., in a Callout) indicating that the addon-kit may receive additional updates post-Storybook 10 release, so developers should refer to the latest
mainbranch for the most current reference implementation.
1-366: Documentation structure and migration clarity are strong; minor enhancements suggested above.Overall, the migration guide is comprehensive and well-organized. The progression from dependency updates → build configuration → local loading covers the essential steps. The diffs are clear and use proper syntax highlighting. The explanatory text between diffs helps orient readers. The recommended optional changes provide a clear path for incremental adoption.
As per coding guidelines, documentation under
docs/for significant changes and migration guides has been properly updated to reflect the shift from CJS to ESM in the Storybook addon build pipeline.
Sidnioulz
left a comment
Sidnioulz
left a comment
There was a problem hiding this comment.
Thanks @Dschungelabenteuer :) Good catch
Let's maybe keep "..." as it is, or also adjust tsup diffs in the guide. In any case @jonniebigodes @kylegach feel free to merge this as it is!
jonniebigodes
changed the title
tsconfig.json diffs in addons migration guidetsconfig.json diffs in addons migration guide
|
Merging this. The unit test issues are unrelated to this pull request, as this is a documentation-only change. @Sidnioulz appreciate the review on it and @Dschungelabenteuer for the time you took to help us with the documentation 🙏 |
4 participants
What I did
The addon migration guide for 10.x.x was very helpful and straightforward. However, it seems that diffs on the
tsconfig.jsonfile are based on changes that only occurred in the recent months (e.g. this commit that changes themodulefield fromcommonjstoPreserve). Those changes do not appear in the diffs despite being quite important.Some of the kit-based addons (if not most) are older than these few months, and I'd guess such addons still have outdated critical fields (like the
moduleone I just mentioned). While debugging these missing changes was pretty quick and easy, it still felt like a small obstacle to the migration!I think those small changes could help a bit with that migration. And as for the addons that were created within these few months (which I'm pretty sure are a smaller part of the whole ecosystem), they'd just have to ignore those diffs they already pulled!
Also please note:
...with the actual…because I'm an obsessed alt+0133 ladChecklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Well, I'll skip that part because… well… it's pure documentation :D
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
Documentation
Refactor