Skip to content

Commit 06dc54c

Browse files
authored
Merge pull request #36310 from storybookjs/fix/init-vitest-range-stderr
CLI: Fix vitest ERESOLVE on fresh Next.js apps.
2 parents bf83086 + 35f9658 commit 06dc54c

6 files changed

Lines changed: 248 additions & 26 deletions

File tree

‎code/core/src/cli/AddonVitestService.constants.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,20 @@ export const SUPPORTED_FRAMEWORKS: readonly SupportedFramework[] = [
1414
SupportedFramework.WEB_COMPONENTS_VITE,
1515
SupportedFramework.TANSTACK_REACT,
1616
];
17+
18+
/**
19+
* The `@types/node` optional peer range declared by the latest Vitest major (Vitest 5.0.0 dropped
20+
* `^20`). A project whose own `@types/node` range does not intersect this range (e.g.
21+
* create-next-app scaffolds pinning `@types/node@^20`) hard-fails npm's peer resolution for an
22+
* unpinned Vitest install, so those projects get VITEST_FALLBACK_SPECIFIER instead. Keep this in
23+
* sync with the latest Vitest major's peerDependencies.
24+
*/
25+
export const LATEST_VITEST_TYPES_NODE_PEER = '^22.0.0 || >=24.0.0';
26+
27+
/**
28+
* Vitest range installed when the project does not declare its own Vitest version AND its
29+
* `@types/node` range conflicts with LATEST_VITEST_TYPES_NODE_PEER. `^4` is the family
30+
* @storybook/addon-vitest's devDependencies are tested against. Keep this in sync with the
31+
* addon's supported majors.
32+
*/
33+
export const VITEST_FALLBACK_SPECIFIER = '^4';

‎code/core/src/cli/AddonVitestService.test.ts‎

Lines changed: 87 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import * as find from 'empathic/find';
1212
import type { ResultPromise } from 'execa';
1313

1414
import { SupportedBuilder, SupportedFramework } from '../types/index.ts';
15-
import { AddonVitestService } from './AddonVitestService.ts';
15+
import { AddonVitestService, canInstallLatestVitest } from './AddonVitestService.ts';
1616

1717
vi.mock('node:fs/promises', { spy: true });
1818
vi.mock('node:os', { spy: true });
@@ -61,11 +61,12 @@ describe('AddonVitestService', () => {
6161
it('should collect base packages when not installed', async () => {
6262
const deps = await service.collectDependencies();
6363

64-
expect(deps).toContain('vitest');
65-
// The addon always installs the Vitest 4 browser provider
66-
expect(deps).toContain('@vitest/browser-playwright');
64+
// No vitest version is declared and no @types/node conflict exists, so the fresh install
65+
// resolves latest Vitest
66+
expect(deps).toContain('vitest@latest');
67+
expect(deps).toContain('@vitest/browser-playwright@latest');
6768
expect(deps).toContain('playwright');
68-
expect(deps).toContain('@vitest/coverage-v8');
69+
expect(deps).toContain('@vitest/coverage-v8@latest');
6970
});
7071

7172
it('should not include base packages if already installed', async () => {
@@ -92,11 +93,10 @@ describe('AddonVitestService', () => {
9293
const deps = await service.collectDependencies();
9394

9495
// Should only contain base packages, not framework-specific ones
95-
expect(deps).toContain('vitest');
96-
// The addon always installs the Vitest 4 browser provider
97-
expect(deps).toContain('@vitest/browser-playwright');
96+
expect(deps).toContain('vitest@latest');
97+
expect(deps).toContain('@vitest/browser-playwright@latest');
9898
expect(deps).toContain('playwright');
99-
expect(deps).toContain('@vitest/coverage-v8');
99+
expect(deps).toContain('@vitest/coverage-v8@latest');
100100
expect(deps.every((d) => !d.includes('nextjs-vite'))).toBe(true);
101101
});
102102

@@ -143,14 +143,88 @@ describe('AddonVitestService', () => {
143143
);
144144
});
145145

146-
it('does not pin anything when the vitest version cannot be resolved', async () => {
146+
it('falls back to the Vitest 4 family when no vitest is declared and @types/node conflicts', async () => {
147147
vi.mocked(mockPackageManager.getDeclaredVersionSpecifier).mockResolvedValue(null);
148+
vi.mocked(mockPackageManager.getAllDependencies).mockReturnValue({
149+
'@types/node': '^20', // e.g. create-next-app scaffolds
150+
});
151+
152+
const deps = await service.collectDependencies();
153+
154+
expect(deps).toContain('vitest@^4');
155+
expect(deps).toContain('@vitest/browser-playwright@^4');
156+
expect(deps).toContain('@vitest/coverage-v8@^4');
157+
expect(deps).toContain('playwright'); // playwright is versioned independently
158+
// The fallback flows through the same alignment path as a declared specifier, so the
159+
// package manager applies its own convention (e.g. pnpm catalog registration).
160+
expect(mockPackageManager.applyVersionToRelatedPackages).toHaveBeenCalledWith(
161+
['vitest', '@vitest/browser-playwright', '@vitest/coverage-v8'],
162+
'^4',
163+
'vitest'
164+
);
165+
});
166+
167+
it('falls back instead of blowing up collectDependencies on a pnpm catalog @types/node', async () => {
168+
vi.mocked(mockPackageManager.getDeclaredVersionSpecifier).mockResolvedValue(null);
169+
vi.mocked(mockPackageManager.getAllDependencies).mockReturnValue({
170+
'@types/node': 'catalog:', // pnpm catalog reference; unparseable as a semver range
171+
});
172+
173+
const deps = await service.collectDependencies();
174+
175+
expect(deps).toContain('vitest@^4');
176+
expect(deps).toContain('@vitest/browser-playwright@^4');
177+
});
178+
179+
it('installs latest Vitest when no vitest is declared and no peer conflict exists', async () => {
180+
vi.mocked(mockPackageManager.getDeclaredVersionSpecifier).mockResolvedValue(null);
181+
vi.mocked(mockPackageManager.getAllDependencies).mockReturnValue({});
148182

149183
const deps = await service.collectDependencies();
150184

151-
expect(deps).toContain('@vitest/coverage-v8');
152-
expect(deps.every((d) => !d.includes('@catalog:') && !d.includes('@3.'))).toBe(true);
153-
expect(mockPackageManager.applyVersionToRelatedPackages).not.toHaveBeenCalled();
185+
expect(deps).toContain('vitest@latest');
186+
expect(deps).toContain('@vitest/browser-playwright@latest');
187+
expect(deps).toContain('@vitest/coverage-v8@latest');
188+
expect(deps).toContain('playwright');
189+
expect(mockPackageManager.applyVersionToRelatedPackages).toHaveBeenCalledWith(
190+
['vitest', '@vitest/browser-playwright', '@vitest/coverage-v8'],
191+
'latest',
192+
'vitest'
193+
);
194+
});
195+
196+
it('installs latest Vitest when the project @types/node range satisfies the latest peer', async () => {
197+
vi.mocked(mockPackageManager.getDeclaredVersionSpecifier).mockResolvedValue(null);
198+
vi.mocked(mockPackageManager.getAllDependencies).mockReturnValue({
199+
'@types/node': '^22',
200+
});
201+
202+
const deps = await service.collectDependencies();
203+
204+
expect(deps).toContain('vitest@latest');
205+
});
206+
});
207+
208+
describe('canInstallLatestVitest', () => {
209+
it('allows latest when @types/node is absent or unversioned', () => {
210+
expect(canInstallLatestVitest({})).toBe(true);
211+
expect(canInstallLatestVitest({ '@types/node': '*' })).toBe(true);
212+
});
213+
214+
it('blocks latest when the @types/node range cannot satisfy the latest peer range', () => {
215+
expect(canInstallLatestVitest({ '@types/node': '^20' })).toBe(false);
216+
expect(canInstallLatestVitest({ '@types/node': '20.19.0' })).toBe(false);
217+
});
218+
219+
it('allows latest when the @types/node range intersects the latest peer range', () => {
220+
expect(canInstallLatestVitest({ '@types/node': '^22' })).toBe(true);
221+
expect(canInstallLatestVitest({ '@types/node': '>=20' })).toBe(true);
222+
});
223+
224+
it('falls back instead of throwing on unparseable specifiers like pnpm catalog references', () => {
225+
expect(() => canInstallLatestVitest({ '@types/node': 'catalog:' })).not.toThrow();
226+
expect(canInstallLatestVitest({ '@types/node': 'catalog:' })).toBe(false);
227+
expect(canInstallLatestVitest({ '@types/node': 'workspace:*' })).toBe(false);
154228
});
155229
});
156230

‎code/core/src/cli/AddonVitestService.ts‎

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,15 @@ import { logger, prompt } from 'storybook/internal/node-logger';
99
import { ErrorCollector } from 'storybook/internal/telemetry';
1010

1111
import * as find from 'empathic/find';
12-
import { coerce, minVersion, satisfies, validRange } from 'semver';
12+
import { coerce, intersects, minVersion, satisfies, validRange } from 'semver';
1313
import { dedent } from 'ts-dedent';
1414

1515
import { SupportedBuilder, type SupportedFramework } from '../types/index.ts';
16-
import { SUPPORTED_FRAMEWORKS } from './AddonVitestService.constants.ts';
16+
import {
17+
LATEST_VITEST_TYPES_NODE_PEER,
18+
SUPPORTED_FRAMEWORKS,
19+
VITEST_FALLBACK_SPECIFIER,
20+
} from './AddonVitestService.constants.ts';
1721

1822
type Result = {
1923
compatible: boolean;
@@ -26,6 +30,24 @@ export interface AddonVitestCompatibilityOptions {
2630
projectRoot?: string;
2731
}
2832

33+
/**
34+
* Whether an unpinned `latest` Vitest install can resolve for this project. The latest Vitest
35+
* major declares LATEST_VITEST_TYPES_NODE_PEER as an optional `@types/node` peer; a project whose
36+
* own `@types/node` range never intersects that range (e.g. create-next-app scaffolds pinning
37+
* `@types/node@^20`) hard-fails npm's peer resolution, so `latest` must be skipped in favor of
38+
* VITEST_FALLBACK_SPECIFIER. No declared `@types/node` at all means nothing to conflict with.
39+
* Unparseable specifiers (pnpm `catalog:` references) cannot be checked for peer compatibility,
40+
* so they also fall back rather than risk an ERESOLVE — or throw on an invalid comparator.
41+
*/
42+
export const canInstallLatestVitest = (allDependencies: Record<string, string>): boolean => {
43+
const typesNodeRange = allDependencies['@types/node'];
44+
if (!typesNodeRange) {
45+
return true;
46+
}
47+
const range = validRange(typesNodeRange);
48+
return range ? intersects(range, LATEST_VITEST_TYPES_NODE_PEER) : false;
49+
};
50+
2951
/**
3052
* Centralized service for @storybook/addon-vitest dependency collection and compatibility
3153
* validation
@@ -68,7 +90,13 @@ export class AddonVitestService {
6890

6991
// Resolve the Vitest version/range to keep the derived `@vitest/*` packages on a compatible
7092
// major. The package manager owns the resolution (e.g. reading a pnpm `catalog:` reference).
71-
const vitestVersionSpecifier = await this.packageManager.getDeclaredVersionSpecifier('vitest');
93+
// Projects declaring no Vitest at all install `latest`, unless the project's `@types/node`
94+
// range conflicts with the latest Vitest major's `@types/node` peer (e.g. create-next-app's
95+
// `@types/node@^20` vs. Vitest 5's `^22.0.0 || >=24.0.0`) — those fall back to the Vitest 4
96+
// family, which is what the addon's devDependencies are tested against.
97+
const vitestVersionSpecifier =
98+
(await this.packageManager.getDeclaredVersionSpecifier('vitest')) ??
99+
(canInstallLatestVitest(allDeps) ? 'latest' : VITEST_FALLBACK_SPECIFIER);
72100

73101
// only install these dependencies if they are not already installed
74102
const basePackages = ['vitest', 'playwright', '@vitest/browser-playwright'];
@@ -90,10 +118,6 @@ export class AddonVitestService {
90118
dependencies.push('@vitest/coverage-v8');
91119
}
92120

93-
if (!vitestVersionSpecifier) {
94-
return dependencies;
95-
}
96-
97121
// Pin the vitest-related packages to the resolved vitest version, letting the package manager
98122
// apply its own convention (e.g. registering pnpm catalog entries). Playwright is versioned
99123
// independently, so it is left untouched.

‎code/core/src/common/js-package-manager/JsPackageManager.test.ts‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import { beforeEach, describe, expect, it, vi } from 'vitest';
22

3+
import { logger, prompt } from 'storybook/internal/node-logger';
4+
35
import { JsPackageManager } from './JsPackageManager.ts';
46

7+
vi.mock('storybook/internal/node-logger', { spy: true });
8+
59
const mockVersions = vi.hoisted(() => ({
610
'@storybook/react': '8.3.0',
711
}));
@@ -88,4 +92,73 @@ describe('JsPackageManager', () => {
8892
expect(result).toEqual(['some-other-package']);
8993
});
9094
});
95+
96+
describe('installDependencies method', () => {
97+
it('propagates a truncated npm stderr tail into the thrown error', async () => {
98+
const stderr = [
99+
'npm error code ERESOLVE',
100+
...Array.from({ length: 30 }, (_, i) => `npm error while resolving line ${i}`),
101+
'npm error Could not resolve dependency: vitest@5.0.0',
102+
].join('\n');
103+
vi.mocked(prompt.executeTaskWithSpinner).mockRejectedValue(
104+
Object.assign(new Error('Command failed: npm install'), { stderr })
105+
);
106+
107+
const error = await jsPackageManager.installDependencies().then(
108+
() => null,
109+
(e) => e
110+
);
111+
112+
// The tail (last 15 lines) is folded into the error message...
113+
expect(error.message).toContain('Could not resolve dependency: vitest@5.0.0');
114+
// ...truncated: the head of a long stderr does not leak into the message...
115+
expect(error.message).not.toContain('npm error code ERESOLVE');
116+
// ...and is printed so failures behind the spinner name themselves.
117+
expect(vi.mocked(logger.error)).toHaveBeenCalledWith(expect.stringContaining('vitest@5.0.0'));
118+
});
119+
120+
it('logs and folds a short stderr tail into the thrown error', async () => {
121+
const stderr = [
122+
'npm error code ERESOLVE',
123+
'npm error Could not resolve dependency: vitest@5.0.0',
124+
].join('\n');
125+
vi.mocked(prompt.executeTaskWithSpinner).mockRejectedValue(
126+
Object.assign(new Error('Command failed: npm install'), { stderr })
127+
);
128+
129+
const error = await jsPackageManager.installDependencies().then(
130+
() => null,
131+
(e) => e
132+
);
133+
134+
// Short outputs are surfaced too — not silently dropped because they fit the tail window.
135+
expect(vi.mocked(logger.error)).toHaveBeenCalledWith(expect.stringContaining('ERESOLVE'));
136+
expect(error.message).toContain('Could not resolve dependency: vitest@5.0.0');
137+
});
138+
139+
it('does not duplicate the tail when the error message already contains it', async () => {
140+
// A plain Error with no structured stderr: getErrorLogs falls back to the message itself,
141+
// so the tail equals the message and appending would duplicate it.
142+
const message =
143+
'Command failed: npm install\nnpm error code ERESOLVE\nnpm error Could not resolve dependency: vitest@5.0.0';
144+
vi.mocked(prompt.executeTaskWithSpinner).mockRejectedValue(new Error(message));
145+
146+
const error = await jsPackageManager.installDependencies().then(
147+
() => null,
148+
(e) => e
149+
);
150+
151+
// The tail is still printed, but not appended a second time.
152+
expect(vi.mocked(logger.error)).toHaveBeenCalledWith(expect.stringContaining('ERESOLVE'));
153+
expect(error.message).toBe(message);
154+
});
155+
156+
it('resolves and clears the installed version cache on success', async () => {
157+
const clearSpy = vi.spyOn(jsPackageManager, 'clearInstalledVersionCache');
158+
vi.mocked(prompt.executeTaskWithSpinner).mockResolvedValue(undefined);
159+
160+
await expect(jsPackageManager.installDependencies()).resolves.toBeUndefined();
161+
expect(clearSpy).toHaveBeenCalled();
162+
});
163+
});
91164
});

‎code/core/src/common/js-package-manager/JsPackageManager.ts‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { findFilesUp, getProjectRoot } from '../utils/paths.ts';
2020
import storybookPackagesVersions from '../versions.ts';
2121
import type { PackageJson, PackageJsonWithDepsAndDevDeps } from './PackageJson.ts';
2222
import type { InstallationMetadata } from './types.ts';
23+
import { getInstallErrorTail } from './util.ts';
2324
import { getVitePlusVersions } from './vite-plus-versions.ts';
2425

2526
export enum PackageManagerName {
@@ -185,12 +186,26 @@ export abstract class JsPackageManager {
185186
}
186187

187188
async installDependencies(options?: { force?: boolean }) {
188-
await prompt.executeTaskWithSpinner(() => this.runInstall(options), {
189-
id: 'install-dependencies',
190-
intro: 'Installing dependencies...',
191-
error: 'Installation of dependencies failed!',
192-
success: 'Dependencies installed',
193-
});
189+
try {
190+
await prompt.executeTaskWithSpinner(() => this.runInstall(options), {
191+
id: 'install-dependencies',
192+
intro: 'Installing dependencies...',
193+
error: 'Installation of dependencies failed!',
194+
success: 'Dependencies installed',
195+
});
196+
} catch (error) {
197+
// The spinner only shows a generic message, and callers like the init flow continue without
198+
// printing the thrown error. Surface the captured output tail so package-manager errors
199+
// (e.g. an npm ERESOLVE) name themselves instead of surfacing later as a missing binary.
200+
const tail = getInstallErrorTail(error);
201+
if (tail) {
202+
logger.error(tail);
203+
if (error instanceof Error && !error.message.includes(tail)) {
204+
error.message = `${error.message}\n\n${tail}`;
205+
}
206+
}
207+
throw error;
208+
}
194209

195210
// Clear installed version cache after installation
196211
this.clearInstalledVersionCache();

‎code/core/src/common/js-package-manager/util.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,3 +206,22 @@ export const getErrorLogs = (error: unknown): string => {
206206

207207
return String(error);
208208
};
209+
210+
/** Number of trailing output lines surfaced when a package install fails. */
211+
export const INSTALL_ERROR_TAIL_LINES = 15;
212+
213+
/**
214+
* The tail of a failed package install's captured output (see `getErrorLogs`). Output longer than
215+
* `lines` lines is truncated to the last `lines` — package managers print their own error summary
216+
* (e.g. an npm ERESOLVE explanation) at the end. Returns '' only when there is no captured output
217+
* at all; short outputs are returned in full so they are still logged and folded into the error.
218+
*/
219+
export const getInstallErrorTail = (error: unknown, lines: number = INSTALL_ERROR_TAIL_LINES) => {
220+
const output = getErrorLogs(error).trimEnd();
221+
if (!output) {
222+
return '';
223+
}
224+
225+
const allLines = output.split('\n');
226+
return allLines.slice(-lines).join('\n').trim();
227+
};

0 commit comments

Comments
 (0)