Skip to content

CLI: Fix vitest ERESOLVE on fresh Next.js apps. - #36310

Merged
ghengeveld merged 4 commits into
nextfrom
fix/init-vitest-range-stderr
Sep 15, 2026
Merged

ghengeveld merged 4 commits into
nextfrom
fix/init-vitest-range-stderr

Conversation

@obvious-autobuild

@obvious-autobuild obvious-autobuild Bot commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Closes #

What I did

The daily init-empty-nextjs-ts CI job has been failing with sh: 1: storybook: not found (exit 127) in the "Run storybook smoke test" step. Root cause chain:

  1. AddonVitestService.collectDependencies() pushed an unpinned vitest into the base dependency set when a fresh project does not declare vitest.
  2. npm resolved vitest 5.0.0, whose optional @types/node peer is ^22.0.0 || >=24.0.0 (Vitest 5 dropped ^20).
  3. create-next-app scaffolds pin @types/node@^20 — npm hard-fails with ERESOLVE.
  4. JsPackageManager.installDependencies runs npm behind a spinner and swallowed the captured stderr on failure, so storybook init exited 0 with no Storybook binary installed — surfacing only later as the smoke test's exit 127.

Two coupled fixes, both in the CLI core:

Peer-aware Vitest selection (AddonVitestService.ts). When a project declares vitest, the declared specifier is preserved unchanged (including pnpm catalog: references). When it does not, the CLI now installs vitest@latest by default and falls back to the ^4 family — the line addon-vitest's own devDependencies test against — when the project's @types/node range conflicts with Vitest 5's peer requirement. The conflict check (canInstallLatestVitest) intersects the project's declared @types/node (from dependencies, devDependencies, and peerDependencies) with LATEST_VITEST_TYPES_NODE_PEER (^22.0.0 || >=24.0.0) using semver.intersects. The selected specifier flows through the existing related-package alignment so vitest, @vitest/browser-playwright, and @vitest/coverage-v8 all resolve to the same family (one source of truth), while playwright stays independently unpinned.

Surface npm stderr on install failure (JsPackageManager / package-manager utils). A new getInstallErrorTail utility returns the last INSTALL_ERROR_TAIL_LINES (15) lines of the captured package-manager output. installDependencies now logs that tail and appends it to the thrown error message when an install exits non-zero, so the next ERESOLVE names itself instead of surfacing as a missing binary. Successful installs still clear the installed-version cache as before.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • unit tests
    • AddonVitestService.test.ts: latest selection with no @types/node; ^4 fallback for @types/node@^20; latest selection for @types/node@^22; canInstallLatestVitest boundary cases; declared specifiers preserved; derived @vitest/* alignment.
    • JsPackageManager.test.ts: truncated npm stderr propagation on failed installs, logger output, and cache clearing on success.

Manual testing

Caution

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Reproduced end-to-end against a local Verdaccio registry, mirroring the CI init-empty-nextjs-ts job recipe:

  1. yarn local-registry --open --publish (proxy on 6001 redirecting @storybook/* to Verdaccio on 6002, other packages to npmjs).
  2. Health-gated the published packs (storybook tarball contains the fixed collectDependencies bundle and dist/bin/dispatcher.js; no stale detectPnp code in create-storybook).
  3. In a scratch dir with CI=1 STORYBOOK_INIT_EMPTY_TYPE=nextjs-ts npm_config_registry=http://localhost:6001: npx --yes storybook init --yes --package-manager npm.

Result (all assertions pass):

  • init exit code: 0
  • node_modules/.bin/storybook: present (the smoke test would run — no more exit 127)
  • Resolved vitest in the scaffold's node_modules: 4.1.11 (the ^4 family, not 5.x)
  • Scaffold package.json: vitest: ^4, @vitest/browser-playwright: ^4, @types/node: ^20 — no ERESOLVE, no "Installation of dependencies failed!"

Pre-fix, the same flow exited 0 with the binary missing and vitest: latest in the scaffold, reproducing the CI failure shape exactly.

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update 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
  • Declare whether manual QA will be needed for this PR during the next release, through qa:needed or qa:skip
  • This PR contains the bug, ci:normal, and qa:skip labels (manual QA waived: CLI dependency-selection change covered by unit tests + the Verdaccio repro above)

Post-merge: init-empty-nextjs-ts must go green in the next CircleCI daily run — PR CI alone does not exercise that job.

🔗 Obvious Project · 🧵 Obvious Thread

@obvious-autobuild obvious-autobuild Bot added bug ci:normal Run our default set of CI jobs (choose this for most PRs). qa:skip Pull Requests that do not need any QA. (e.g. documentation) labels Sep 14, 2026
@valentinpalkovic

Copy link
Copy Markdown
Contributor

@ObviousApp I dislike the solution. The solution should be to only add vitest latest if @types/node peer dependency requirements are not blocked (@types/node` >= 22) or not existent. in the case of nextjs, vitest 4 should be installed. So the default should still be vitest 5, but with a fallback to vitest 4, if it cannot be installed due to conflicting peer dependency requirements!

@obvious-autobuild

Copy link
Copy Markdown
Contributor Author

Replying to this comment

Fair — implemented in 72c2b79. The default for fresh installs is back to latest; ^4 is now only a fallback, selected by a small predicate (canInstallLatestVitest): if the project declares no @types/node at all, or its declared range intersects the latest vitest major's optional @types/node peer (^22.0.0 || >=24.0.0), we install latest; if the range cannot satisfy that peer (create-next-app@16.3.x pinning @types/node@^20), we install vitest@^4 and align the derived @vitest/* packages to ^4 through the same single alignment path as before. Declared-vitest projects are untouched. Suite updated accordingly (64 passing); re-running the local verdaccio end-to-end repro against rebuilt packages now.

@obvious-autobuild
obvious-autobuild Bot marked this pull request as ready for review September 14, 2026 16:07
@ghengeveld

Copy link
Copy Markdown
Member

@ObviousApp Please consider these issues. Verify before fixing.

A pnpm app with @types/node: catalog: and no vitest blows up collectDependencies and skips the whole Vitest install. In canInstallLatestVitest, semver throws "Invalid comparator" because it runs on the raw getAllDependencies() map rather than the resolved specifier. We should probably add a validRange guard and include a test for this case.

getInstallErrorTail returns '' whenever the captured output is 15 lines or less, then JsPackageManager skips both logger.error and the message append. Always log the last 15 lines; only skip appending when error.message already contains 15 lines.

When storybook init runs on a project that declares no vitest version
(fresh create-next-app@16.3.x scaffolds), collectDependencies() pushed an
unpinned `vitest`, which now resolves vitest@5.0.0. Vitest 5 dropped
`^20` from its optional `@types/node` peer range, so npm hard-fails with
ERESOLVE against the scaffold's `@types/node@^20` pin, and the install
silently fails: init exits 0 with no storybook binary.

Default the collected vitest family to ^4 (what addon-vitest's
devDependencies are tested against) through the existing
applyVersionToRelatedPackages alignment, so vitest,
@vitest/browser-playwright and @vitest/coverage-v8 stay on one range and
the declared-specifier path (including pnpm catalog: references) is
untouched.
JsPackageManager.installDependencies runs the install behind a spinner
and the captured package-manager output never reaches the caller: the
spinner prints a generic failure message, and flows like `storybook
init` swallow the thrown error and continue. A failed install therefore
surfaces only indirectly — e.g. `sh: 1: storybook: not found` from a
later smoke test, exit 127, with no hint that npm ERESOLVE was the cause.

Propagate a truncated output tail (last 15 lines, where npm prints its
own error summary) into the thrown error message and log it, so the next
dependency-resolution failure names itself.
Address review feedback: keep unpinned (latest) vitest as the default for
fresh installs, and only fall back to the vitest ^4 family when the
project's declared @types/node range conflicts with the latest vitest
major's optional @types/node peer (^22.0.0 || >=24.0.0) — e.g.
create-next-app scaffolds pinning @types/node@^20.
Review feedback on #36310:

- canInstallLatestVitest: validate the @types/node range with semver
  validRange before intersecting. A pnpm `catalog:` reference threw
  "Invalid comparator" and aborted the whole Vitest install; unparseable
  specifiers now fall back to the ^4 family.
- getInstallErrorTail: return the tail even when the captured output
  fits the 15-line window. Short but real package-manager errors were
  returned as '' and never logged or folded into the thrown error.
  The dedup (skip appending when the message already contains the tail)
  stays at the JsPackageManager call site.
@obvious-autobuild
obvious-autobuild Bot force-pushed the fix/init-vitest-range-stderr branch from 72c2b79 to 35f9658 Compare September 15, 2026 06:51
@obvious-autobuild

Copy link
Copy Markdown
Contributor Author

Replying to this comment

Both verified against the code and fixed in 35f9658 — thank you.

  1. pnpm catalog: crash — confirmed: canInstallLatestVitest ran semver.intersects on the raw getAllDependencies() value, so @types/node: catalog: threw Invalid comparator inside collectDependencies and skipped the whole Vitest install. The predicate now runs the specifier through semver.validRange first; unparseable specifiers (catalog/workspace references) cannot be checked for peer compatibility, so they fall back to the ^4 family. Added predicate tests (catalog:, workspace:*) and a collectDependencies regression test asserting vitest@^4 is collected with a catalog: @types/node.

  2. getInstallErrorTail dropping short output — confirmed: the utility returned `` when the whole output fit the 15-line window, and the call sites if (tail) then skipped both `logger.error` and the message append. The utility now returns the last ≤15 lines whenever there is captured output at all; `JsPackageManager.installDependencies` always logs it, and only skips appending when `error.message` already contains the tail (dedup stays at the call site). Added tests for the short-output path (logged and folded into the error) and for the already-contains dedup (logged once, message unchanged).

Both suites pass: 77/77 (AddonVitestService.test.ts + JsPackageManager.test.ts), oxlint clean, oxfmt applied. Note: the branch was also rebased onto current next in the same push, so CI is re-running from scratch.

@storybook-app-bot

Copy link
Copy Markdown
Contributor

Package Benchmarks

Commit: 35f9658, ran on 15 September 2026 at 07:01:52 UTC

The following packages have significant changes to their size or dependencies:

@storybook/builder-webpack5

Before After Difference
Dependency count 181 181 0
Self size 92 KB 92 KB 🚨 +48 B 🚨
Dependency size 38.34 MB 38.35 MB 🚨 +12 KB 🚨
Bundle Size Analyzer Link Link

@storybook/angular

Before After Difference
Dependency count 186 186 0
Self size 266 KB 267 KB 🚨 +108 B 🚨
Dependency size 31.37 MB 31.39 MB 🚨 +12 KB 🚨
Bundle Size Analyzer Link Link

@storybook/ember

Before After Difference
Dependency count 180 180 0
Self size 13 KB 13 KB 🎉 -18 B 🎉
Dependency size 33.65 MB 33.66 MB 🚨 +12 KB 🚨
Bundle Size Analyzer Link Link

@storybook/nextjs

Before After Difference
Dependency count 447 447 0
Self size 641 KB 641 KB 0 B
Dependency size 62.25 MB 62.26 MB 🚨 +12 KB 🚨
Bundle Size Analyzer Link Link

@storybook/nextjs-vite

Before After Difference
Dependency count 101 101 0
Self size 1.42 MB 1.42 MB 🚨 +48 B 🚨
Dependency size 23.53 MB 23.54 MB 🚨 +12 KB 🚨
Bundle Size Analyzer Link Link

@storybook/react-native-web-vite

Before After Difference
Dependency count 121 121 0
Self size 29 KB 29 KB 🚨 +18 B 🚨
Dependency size 25.43 MB 25.44 MB 🚨 +12 KB 🚨
Bundle Size Analyzer Link Link

@storybook/react-vite

Before After Difference
Dependency count 79 79 0
Self size 32 KB 32 KB 0 B
Dependency size 20.64 MB 20.66 MB 🚨 +12 KB 🚨
Bundle Size Analyzer Link Link

@storybook/react-webpack5

Before After Difference
Dependency count 266 266 0
Self size 23 KB 23 KB 🎉 -12 B 🎉
Dependency size 50.87 MB 50.89 MB 🚨 +12 KB 🚨
Bundle Size Analyzer Link Link

@storybook/server-webpack5

Before After Difference
Dependency count 193 193 0
Self size 15 KB 15 KB 🚨 +12 B 🚨
Dependency size 39.61 MB 39.62 MB 🚨 +12 KB 🚨
Bundle Size Analyzer Link Link

@storybook/svelte-vite

Before After Difference
Dependency count 11 11 0
Self size 54 KB 54 KB 0 B
Dependency size 25.54 MB 25.55 MB 🚨 +12 KB 🚨
Bundle Size Analyzer Link Link

@storybook/sveltekit

Before After Difference
Dependency count 12 12 0
Self size 56 KB 56 KB 🎉 -24 B 🎉
Dependency size 25.59 MB 25.61 MB 🚨 +12 KB 🚨
Bundle Size Analyzer Link Link

@storybook/tanstack-react

Before After Difference
Dependency count 80 80 0
Self size 132 KB 132 KB 0 B
Dependency size 20.68 MB 20.69 MB 🚨 +12 KB 🚨
Bundle Size Analyzer Link Link

@storybook/vue3-vite

Before After Difference
Dependency count 94 94 0
Self size 31 KB 31 KB 0 B
Dependency size 18.64 MB 18.65 MB 🚨 +12 KB 🚨
Bundle Size Analyzer Link Link

@storybook/preset-react-webpack

Before After Difference
Dependency count 152 152 0
Self size 14 KB 14 KB 🚨 +18 B 🚨
Dependency size 34.99 MB 35.00 MB 🚨 +12 KB 🚨
Bundle Size Analyzer Link Link

@obvious-autobuild

Copy link
Copy Markdown
Contributor Author

Replying to this comment

Acknowledged — the deltas here are uniform across every package (dependency count unchanged, dependency size +12 KB, self-size shifts of tens of bytes), which is the rebased core (this branch now tracks current next) being re-bundled into each framework, not the two-file CLI change in this PR (AddonVitestService.ts and the package-manager utils are not part of any framework bundle). No action needed; flagging in case the core-size drift matters to the next-vs-base comparison.

@ghengeveld
ghengeveld merged commit 06dc54c into next Sep 15, 2026
154 checks passed
@ghengeveld
ghengeveld deleted the fix/init-vitest-range-stderr branch September 15, 2026 07:51
ghengeveld pushed a commit that referenced this pull request Sep 18, 2026
Review feedback on #36310:

- canInstallLatestVitest: validate the @types/node range with semver
  validRange before intersecting. A pnpm `catalog:` reference threw
  "Invalid comparator" and aborted the whole Vitest install; unparseable
  specifiers now fall back to the ^4 family.
- getInstallErrorTail: return the tail even when the captured output
  fits the 15-line window. Short but real package-manager errors were
  returned as '' and never logged or folded into the thrown error.
  The dedup (skip appending when the message already contains the tail)
  stays at the JsPackageManager call site.

(cherry picked from commit 35f9658)
@ghengeveld ghengeveld added the patch:yes Bugfix & documentation PR that need to be picked to main branch label Sep 29, 2026
@github-actions github-actions Bot mentioned this pull request Sep 29, 2026
3 tasks done
ghengeveld added a commit that referenced this pull request Sep 29, 2026
CLI: Fix vitest ERESOLVE on fresh Next.js apps.
(cherry picked from commit 06dc54c)
ghengeveld added a commit that referenced this pull request Sep 29, 2026
CLI: Fix vitest ERESOLVE on fresh Next.js apps.
(cherry picked from commit 06dc54c)
@github-actions github-actions Bot added the patch:done Patch/release PRs already cherry-picked to main/release branch label Sep 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug ci:normal Run our default set of CI jobs (choose this for most PRs). patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch qa:skip Pull Requests that do not need any QA. (e.g. documentation)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants