Skip to content

Commit a9da81c

Browse files
Merge branch 'valentin/cli-init-rework' into valentin/cli-init-docs-performance
2 parents 31bda49 + bada431 commit a9da81c

File tree

24 files changed

+521
-416
lines changed

24 files changed

+521
-416
lines changed

code/addons/vitest/src/postinstall.ts

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -324,29 +324,30 @@ export default async function postInstall(options: PostinstallOptions) {
324324

325325
if (a11yAddon) {
326326
try {
327-
const command = ['automigrate', 'addon-a11y-addon-test'];
328-
329-
command.push('--loglevel', 'silent');
330-
command.push('--yes', '--skip-doctor');
327+
const command = [
328+
'storybook',
329+
'automigrate',
330+
'addon-a11y-addon-test',
331+
'--loglevel=silent',
332+
'--yes',
333+
'--skip-doctor',
334+
];
331335

332336
if (options.packageManager) {
333-
command.push('--package-manager', options.packageManager);
337+
command.push(`--package-manager=${options.packageManager}`);
334338
}
335339

336340
if (options.skipInstall) {
337341
command.push('--skip-install');
338342
}
339343

340344
if (options.configDir !== '.storybook') {
341-
command.push('--config-dir', `"${options.configDir}"`);
345+
command.push(`--config-dir="${options.configDir}"`);
342346
}
343347

344-
const remoteCommand = packageManager.getRemoteRunCommand('storybook', command);
345-
const [cmd, ...args] = remoteCommand.split(' ');
346-
347348
await prompt.executeTask(
348349
// TODO: Remove stdio: 'ignore' once we have a way to log the output of the command properly
349-
() => packageManager.executeCommand({ command: cmd, args, stdio: 'ignore' }),
350+
() => packageManager.runPackageCommand({ args: command, stdio: 'ignore' }),
350351
{
351352
intro: 'Setting up a11y addon for @storybook/addon-vitest',
352353
error: 'Failed to setup a11y addon for @storybook/addon-vitest',

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import * as fs from 'node:fs/promises';
33
import { beforeEach, describe, expect, it, vi } from 'vitest';
44

55
import type { JsPackageManager } from 'storybook/internal/common';
6-
import { getProjectRoot } from 'storybook/internal/common';
6+
import { executeCommand, getProjectRoot } from 'storybook/internal/common';
77
import { logger, prompt } from 'storybook/internal/node-logger';
88

99
import * as find from 'empathic/find';
@@ -30,7 +30,7 @@ describe('AddonVitestService', () => {
3030
mockPackageManager = {
3131
getAllDependencies: vi.fn(),
3232
getInstalledVersion: vi.fn(),
33-
executeCommand: vi.fn(),
33+
runRemoteCommand: vi.fn(),
3434
} as Partial<JsPackageManager> as JsPackageManager;
3535

3636
// Setup default mocks for logger and prompt
@@ -382,14 +382,16 @@ describe('AddonVitestService', () => {
382382
intro: 'Installing Playwright browser binaries',
383383
error: expect.stringContaining('An error occurred'),
384384
success: 'Playwright browser binaries installed successfully',
385+
abortable: true,
385386
});
386387
});
387388

388389
it('should execute playwright install command', async () => {
389-
let commandFactory: (() => ExecaChildProcess) | (() => ExecaChildProcess)[];
390+
type ChildProcessFactory = (signal?: AbortSignal) => ExecaChildProcess;
391+
let commandFactory: ChildProcessFactory | ChildProcessFactory[];
390392
vi.mocked(prompt.confirm).mockResolvedValue(true);
391393
vi.mocked(prompt.executeTaskWithSpinner).mockImplementation(
392-
async (factory: (() => ExecaChildProcess) | (() => ExecaChildProcess)[]) => {
394+
async (factory: ChildProcessFactory | ChildProcessFactory[]) => {
393395
commandFactory = Array.isArray(factory) ? factory[0] : factory;
394396
// Simulate the child process completion
395397
commandFactory();
@@ -398,10 +400,10 @@ describe('AddonVitestService', () => {
398400

399401
await service.installPlaywright(mockPackageManager);
400402

401-
expect(mockPackageManager.executeCommand).toHaveBeenCalledWith({
402-
command: 'npx',
403+
expect(mockPackageManager.runRemoteCommand).toHaveBeenCalledWith({
403404
args: ['playwright', 'install', 'chromium', '--with-deps'],
404-
killSignal: 'SIGINT',
405+
signal: undefined,
406+
stdio: 'ignore',
405407
});
406408
});
407409

code/core/src/cli/AddonVitestService.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type { JsPackageManager } from 'storybook/internal/common';
55
import { getProjectRoot } from 'storybook/internal/common';
66
import { CLI_COLORS } from 'storybook/internal/node-logger';
77
import { logger, prompt } from 'storybook/internal/node-logger';
8+
import { ErrorCollector } from 'storybook/internal/telemetry';
89

910
import type { CallExpression } from '@babel/types';
1011
import * as find from 'empathic/find';
@@ -134,26 +135,25 @@ export class AddonVitestService {
134135

135136
if (shouldBeInstalled) {
136137
await prompt.executeTaskWithSpinner(
137-
() => {
138-
const result = packageManager.executeCommand({
139-
command: 'npx',
138+
(signal) =>
139+
packageManager.runRemoteCommand({
140140
args: playwrightCommand,
141-
killSignal: 'SIGINT',
142-
});
143-
144-
return result;
145-
},
141+
stdio: 'ignore',
142+
signal,
143+
}),
146144
{
147145
id: 'playwright-installation',
148146
intro: 'Installing Playwright browser binaries',
149-
error: `An error occurred while installing Playwright browser binaries. Please run the following command later: ${playwrightCommand.join(' ')}`,
147+
error: `An error occurred while installing Playwright browser binaries. Please run the following command later: npx ${playwrightCommand.join(' ')}`,
150148
success: 'Playwright browser binaries installed successfully',
149+
abortable: true,
151150
}
152151
);
153152
} else {
154153
logger.warn('Playwright installation skipped');
155154
}
156155
} catch (e) {
156+
ErrorCollector.addError(e);
157157
if (e instanceof Error) {
158158
errors.push(e.stack ?? e.message);
159159
} else {

code/core/src/common/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export * from './utils/transform-imports';
4747
export * from '../shared/utils/module';
4848
export * from './utils/get-addon-names';
4949
export * from './utils/utils';
50+
export * from './utils/command';
5051

5152
export { versions };
5253

code/core/src/common/js-package-manager/BUNProxy.ts

Lines changed: 37 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,12 @@ import { logger, prompt } from 'storybook/internal/node-logger';
66
import { FindPackageVersionsError } from 'storybook/internal/server-errors';
77

88
import * as find from 'empathic/find';
9+
// eslint-disable-next-line depend/ban-dependencies
10+
import type { ExecaChildProcess } from 'execa';
911
import sort from 'semver/functions/sort.js';
1012

13+
import type { ExecuteCommandOptions } from '../utils/command';
14+
import { executeCommand, executeCommandSync } from '../utils/command';
1115
import { getProjectRoot } from '../utils/paths';
1216
import { JsPackageManager } from './JsPackageManager';
1317
import type { PackageJson } from './PackageJson';
@@ -69,7 +73,7 @@ export class BUNProxy extends JsPackageManager {
6973
installArgs: string[] | undefined;
7074

7175
async initPackageJson() {
72-
return this.executeCommand({ command: 'bun', args: ['init'] });
76+
return executeCommand({ command: 'bun', args: ['init'] });
7377
}
7478

7579
getRunStorybookCommand(): string {
@@ -84,6 +88,10 @@ export class BUNProxy extends JsPackageManager {
8488
return `bunx ${pkg}${specifier ? `@${specifier}` : ''} ${args.join(' ')}`;
8589
}
8690

91+
public runRemoteCommand(options: Omit<ExecuteCommandOptions, 'command'> & { args: string[] }) {
92+
return executeCommand({ command: 'bunx', ...options });
93+
}
94+
8795
public async getModulePackageJSON(packageName: string): Promise<PackageJson | null> {
8896
const wantedPath = join('node_modules', packageName, 'package.json');
8997
const packageJsonPath = find.up(wantedPath, { cwd: this.cwd, last: getProjectRoot() });
@@ -103,31 +111,25 @@ export class BUNProxy extends JsPackageManager {
103111
return this.installArgs;
104112
}
105113

106-
public runPackageCommandSync(
107-
command: string,
108-
args: string[],
109-
cwd?: string,
110-
stdio?: 'pipe' | 'inherit'
111-
): string {
112-
return this.executeCommandSync({
114+
public runPackageCommandSync({
115+
args,
116+
...options
117+
}: Omit<ExecuteCommandOptions, 'command'> & { args: string[] }): string {
118+
return executeCommandSync({
113119
command: 'bun',
114-
args: ['run', command, ...args],
115-
cwd,
116-
stdio,
120+
args: ['run', ...args],
121+
...options,
117122
});
118123
}
119124

120-
public runPackageCommand(
121-
command: string,
122-
args: string[],
123-
cwd?: string,
124-
stdio?: 'pipe' | 'inherit'
125-
) {
126-
return this.executeCommand({
125+
public runPackageCommand({
126+
args,
127+
...options
128+
}: Omit<ExecuteCommandOptions, 'command'> & { args: string[] }): ExecaChildProcess {
129+
return executeCommand({
127130
command: 'bun',
128-
args: ['run', command, ...args],
129-
cwd,
130-
stdio,
131+
args: ['run', ...args],
132+
...options,
131133
});
132134
}
133135

@@ -137,15 +139,21 @@ export class BUNProxy extends JsPackageManager {
137139
cwd?: string,
138140
stdio?: 'inherit' | 'pipe' | 'ignore'
139141
) {
140-
return this.executeCommand({ command: 'bun', args: [command, ...args], cwd, stdio });
142+
return executeCommand({
143+
command: 'bun',
144+
args: [command, ...args],
145+
cwd: cwd ?? this.cwd,
146+
stdio,
147+
});
141148
}
142149

143150
public async findInstallations(pattern: string[], { depth = 99 }: { depth?: number } = {}) {
144151
const exec = async ({ packageDepth }: { packageDepth: number }) => {
145152
const pipeToNull = platform() === 'win32' ? '2>NUL' : '2>/dev/null';
146-
return this.executeCommand({
153+
return executeCommand({
147154
command: 'npm',
148155
args: ['ls', '--json', `--depth=${packageDepth}`, pipeToNull],
156+
cwd: this.cwd,
149157
env: {
150158
FORCE_COLOR: 'false',
151159
},
@@ -188,7 +196,7 @@ export class BUNProxy extends JsPackageManager {
188196
}
189197

190198
protected runInstall(options?: { force?: boolean }) {
191-
return this.executeCommand({
199+
return executeCommand({
192200
command: 'bun',
193201
args: ['install', ...this.getInstallArgs(), ...(options?.force ? ['--force'] : [])],
194202
cwd: this.cwd,
@@ -197,8 +205,9 @@ export class BUNProxy extends JsPackageManager {
197205
}
198206

199207
public async getRegistryURL() {
200-
const process = this.executeCommand({
208+
const process = executeCommand({
201209
command: 'npm',
210+
cwd: this.cwd,
202211
// "npm config" commands are not allowed in workspaces per default
203212
// https://github.com/npm/cli/issues/6099#issuecomment-1847584792
204213
args: ['config', 'get', 'registry', '-ws=false', '-iwr'],
@@ -215,7 +224,7 @@ export class BUNProxy extends JsPackageManager {
215224
args = ['-D', ...args];
216225
}
217226

218-
return this.executeCommand({
227+
return executeCommand({
219228
command: 'bun',
220229
args: ['add', ...args, ...this.getInstallArgs()],
221230
stdio: 'pipe',
@@ -229,8 +238,9 @@ export class BUNProxy extends JsPackageManager {
229238
): Promise<T extends true ? string[] : string> {
230239
const args = fetchAllVersions ? ['versions', '--json'] : ['version'];
231240
try {
232-
const process = this.executeCommand({
241+
const process = executeCommand({
233242
command: 'npm',
243+
cwd: this.cwd,
234244
args: ['info', packageName, ...args],
235245
});
236246
const result = await process;

0 commit comments

Comments
 (0)