Build: Upgrade type-fest to latest version 5.6.0#34791
Conversation
|
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:
📝 WalkthroughWalkthroughUpgrade type-fest to v5.6.0 across manifests, migrate RemoveIndexSignature → OmitIndexSignature in multiple type-level inference sites, change package.json write logic to mutate fields in-place, and remove two obsolete type-workaround/comments. Changestype-fest v5.6.0 upgrade and type utility migration
🎯 3 (Moderate) | ⏱️ ~20 minutes
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 |
|
I also noticed that type-fest and ts-dedent are actual dependencies for the svelte/vue renderes, but only dev deps for react. Is this on purpose or should I make them dev deps everywhere? |
c936ccf to
ca00b5e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/core/src/cli/eslintPlugin.ts`:
- Line 326: The call to packageManager.writePackageJson(packageJson) uses the
default cwd and can write the ESLint config to the wrong manifest in monorepos;
update the call so writePackageJson explicitly targets the primary package
directory you derived (use the primary package path or primaryPackageJson’s
directory) when persisting packageJson (i.e., invoke
packageManager.writePackageJson with the packageJson and the primary package
path/dir argument or API variant that accepts a target directory) to ensure the
config is written to primaryPackageJson’s package manifest.
🪄 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: d145d017-7d10-4b2c-9ba2-d41e620599cb
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
code/core/package.jsoncode/core/src/cli/dev.tscode/core/src/cli/eslintPlugin.tscode/core/src/common/js-package-manager/JsPackageManager.tscode/core/src/csf/story.tscode/frameworks/angular/src/client/preview.tscode/frameworks/tanstack-react/src/index.tscode/lib/cli-storybook/src/codemod/csf-factories.tscode/renderers/react/package.jsoncode/renderers/react/src/preview.tsxcode/renderers/svelte/package.jsoncode/renderers/vue3/package.jsoncode/renderers/vue3/src/preview.tscode/renderers/vue3/src/public-types.tscode/renderers/web-components/src/preview.tspackage.jsonscripts/package.json
💤 Files with no reviewable changes (1)
- code/lib/cli-storybook/src/codemod/csf-factories.ts
✅ Files skipped from review due to trivial changes (2)
- package.json
- scripts/package.json
🚧 Files skipped from review as they are similar to previous changes (11)
- code/core/package.json
- code/renderers/react/package.json
- code/renderers/svelte/package.json
- code/frameworks/tanstack-react/src/index.ts
- code/renderers/vue3/package.json
- code/renderers/react/src/preview.tsx
- code/renderers/vue3/src/preview.ts
- code/renderers/vue3/src/public-types.ts
- code/core/src/cli/dev.ts
- code/core/src/csf/story.ts
- code/frameworks/angular/src/client/preview.ts
ca00b5e to
92dda65
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/core/src/common/js-package-manager/JsPackageManager.ts`:
- Around line 613-617: The code mutates the shared cached packageJson object
in-place by assigning to packageJson.scripts before calling writePackageJson,
which can leave the in-memory cache inconsistent if the disk write fails; fix by
creating a shallow copy of packageJson (e.g., newPackageJson = { ...packageJson,
scripts: { ...packageJson.scripts, ...scripts } }) and pass that copy to
writePackageJson instead of mutating packageJson; ensure any cache update only
happens after a successful write (update the cached reference to newPackageJson
only once writePackageJson completes) while referring to the packageJson
variable, the scripts variable, and the writePackageJson method in
JsPackageManager.
🪄 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: e2db3171-ec86-4302-b2b2-38c9b542db60
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
code/core/package.jsoncode/core/src/cli/dev.tscode/core/src/cli/eslintPlugin.tscode/core/src/common/js-package-manager/JsPackageManager.tscode/core/src/csf/story.tscode/frameworks/angular/src/client/preview.tscode/frameworks/tanstack-react/src/index.tscode/lib/cli-storybook/src/codemod/csf-factories.tscode/renderers/react/package.jsoncode/renderers/react/src/preview.tsxcode/renderers/svelte/package.jsoncode/renderers/vue3/package.jsoncode/renderers/vue3/src/preview.tscode/renderers/vue3/src/public-types.tscode/renderers/web-components/src/preview.tspackage.jsonscripts/package.json
💤 Files with no reviewable changes (1)
- code/lib/cli-storybook/src/codemod/csf-factories.ts
✅ Files skipped from review due to trivial changes (3)
- scripts/package.json
- package.json
- code/renderers/vue3/src/public-types.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- code/core/src/cli/dev.ts
- code/renderers/svelte/package.json
- code/renderers/web-components/src/preview.ts
- code/core/package.json
- code/renderers/react/package.json
- code/frameworks/angular/src/client/preview.ts
- code/core/src/csf/story.ts
- code/renderers/react/src/preview.tsx
- code/renderers/vue3/src/preview.ts
- code/core/src/cli/eslintPlugin.ts
- code/frameworks/tanstack-react/src/index.ts
| packageJson.scripts = { | ||
| ...packageJson.scripts, | ||
| ...scripts, | ||
| }; | ||
| this.writePackageJson(packageJson, operationDir); |
There was a problem hiding this comment.
Avoid mutating cached packageJson before disk write succeeds.
On Line 613, packageJson is mutated in-place (shared cache object) before writePackageJson. If the write throws, cache state can diverge from disk state.
Suggested fix
public addScripts(scripts: Record<string, string>) {
const { operationDir, packageJson } = this.#getPrimaryPackageJson();
- packageJson.scripts = {
- ...packageJson.scripts,
- ...scripts,
- };
- this.writePackageJson(packageJson, operationDir);
+ const nextPackageJson = {
+ ...packageJson,
+ scripts: {
+ ...packageJson.scripts,
+ ...scripts,
+ },
+ };
+ this.writePackageJson(nextPackageJson, operationDir);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@code/core/src/common/js-package-manager/JsPackageManager.ts` around lines 613
- 617, The code mutates the shared cached packageJson object in-place by
assigning to packageJson.scripts before calling writePackageJson, which can
leave the in-memory cache inconsistent if the disk write fails; fix by creating
a shallow copy of packageJson (e.g., newPackageJson = { ...packageJson, scripts:
{ ...packageJson.scripts, ...scripts } }) and pass that copy to writePackageJson
instead of mutating packageJson; ensure any cache update only happens after a
successful write (update the cached reference to newPackageJson only once
writePackageJson completes) while referring to the packageJson variable, the
scripts variable, and the writePackageJson method in JsPackageManager.
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 72 | 72 | 0 |
| Self size | 20.25 MB | 20.27 MB | 🚨 +19 KB 🚨 |
| Dependency size | 36.17 MB | 36.17 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/angular
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 185 | 185 | 0 |
| Self size | 142 KB | 160 KB | 🚨 +17 KB 🚨 |
| Dependency size | 30.73 MB | 30.73 MB | 🎉 -54 B 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 534 | 534 | 0 |
| Self size | 662 KB | 662 KB | 0 B |
| Dependency size | 61.37 MB | 61.41 MB | 🚨 +34 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 93 | 93 | 0 |
| Self size | 1.38 MB | 1.38 MB | 🎉 -48 B 🎉 |
| Dependency size | 24.79 MB | 24.83 MB | 🚨 +34 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preact-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 14 | 14 | 0 |
| Self size | 13 KB | 13 KB | 🎉 -12 B 🎉 |
| Dependency size | 1.47 MB | 1.49 MB | 🚨 +23 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 122 | 122 | 0 |
| Self size | 30 KB | 30 KB | 0 B |
| Dependency size | 25.86 MB | 25.90 MB | 🚨 +34 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 83 | 83 | 0 |
| Self size | 36 KB | 36 KB | 🚨 +18 B 🚨 |
| Dependency size | 22.57 MB | 22.60 MB | 🚨 +34 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 271 | 271 | 0 |
| Self size | 23 KB | 23 KB | 🚨 +12 B 🚨 |
| Dependency size | 45.91 MB | 45.94 MB | 🚨 +34 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/svelte-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 19 | 20 | 🚨 +1 🚨 |
| Self size | 56 KB | 56 KB | 🎉 -12 B 🎉 |
| Dependency size | 26.65 MB | 27.00 MB | 🚨 +352 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/sveltekit
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 20 | 21 | 🚨 +1 🚨 |
| Self size | 56 KB | 56 KB | 0 B |
| Dependency size | 26.71 MB | 27.06 MB | 🚨 +352 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/tanstack-react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 84 | 84 | 0 |
| Self size | 107 KB | 106 KB | 🎉 -1 KB 🎉 |
| Dependency size | 22.60 MB | 22.64 MB | 🚨 +34 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/vue3-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 108 | 109 | 🚨 +1 🚨 |
| Self size | 36 KB | 36 KB | 0 B |
| Dependency size | 43.75 MB | 44.10 MB | 🚨 +352 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/web-components-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 15 | 15 | 0 |
| Self size | 19 KB | 19 KB | 🚨 +12 B 🚨 |
| Dependency size | 1.52 MB | 1.54 MB | 🚨 +17 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 203 | 203 | 0 |
| Self size | 908 KB | 908 KB | 🎉 -55 B 🎉 |
| Dependency size | 87.56 MB | 87.58 MB | 🚨 +19 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 196 | 196 | 0 |
| Self size | 32 KB | 32 KB | 🚨 +36 B 🚨 |
| Dependency size | 86.05 MB | 86.06 MB | 🚨 +19 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 73 | 73 | 0 |
| Self size | 1.08 MB | 1.08 MB | 🎉 -66 B 🎉 |
| Dependency size | 56.43 MB | 56.45 MB | 🚨 +19 KB 🚨 |
| Bundle Size Analyzer | node | node |
@storybook/preact
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 2 | 2 | 0 |
| Self size | 23 KB | 46 KB | 🚨 +23 KB 🚨 |
| Dependency size | 32 KB | 32 KB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 59 | 59 | 0 |
| Self size | 1.47 MB | 1.51 MB | 🚨 +34 KB 🚨 |
| Dependency size | 13.30 MB | 13.30 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/svelte
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 2 | 3 | 🚨 +1 🚨 |
| Self size | 49 KB | 49 KB | 🚨 +1 B 🚨 |
| Dependency size | 230 KB | 582 KB | 🚨 +352 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/vue3
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 3 | 4 | 🚨 +1 🚨 |
| Self size | 66 KB | 66 KB | 🚨 +7 B 🚨 |
| Dependency size | 213 KB | 565 KB | 🚨 +352 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/web-components
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 3 | 3 | 0 |
| Self size | 62 KB | 79 KB | 🚨 +17 KB 🚨 |
| Dependency size | 47 KB | 47 KB | 0 B |
| Bundle Size Analyzer | Link | Link |
|
I cannot reproduce the failing ts failures locally. :( |
It seems to be used in react public API files though. It probably needs to be moved as |
The root `resolutions` field forced every package in the tree onto a single type-fest version. Raising it to v5 broke external packages whose typings target older type-fest majors (e.g. react-joyride, whose types rely on type-fest v2-era utilities). Drop the global `type-fest` resolution so each package resolves the version it declares, and scope react-joyride to type-fest v2 — the version it was effectively pinned to before this upgrade. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`writePackageJson` defaults to `cwd`; pass the primary package's `operationDir` so the storybook ESLint config is written to the correct manifest in monorepos. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Picked this up to get it merge-ready. Summary of what I changed on top of the original type-fest bump:
|
Reflect the type-fest resolution change in the ecosystem-ci `EXISTING_RESOLUTIONS` set so it stays in sync with the root package.json `resolutions` keys. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thanks @valentinpalkovic! |
What I did
Update type-fest. Triggered by
when using rolldown (see tobiasdiez/storybook-vue-addon#238).
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
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 canary-release-pr.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
Chores
Bug Fixes / Improvements