Merge pull request #789 from chromaui/ghengeveld/ap-2652-support-pnpm…

…-in-our-cli

Use `@antfu/ni` to resolve build command

.github/workflows/smoke-test-node14.yml

@@ -8,9 +8,9 @@ jobs:
       - uses: actions/checkout@v2
         with:
           fetch-depth: 0
-      - uses: actions/setup-node@v2
+      - uses: actions/setup-node@v3
         with:
-          node-version: '14'
+          node-version: 14
       - run: yarn
       - run: yarn build
       - uses: ./

.github/workflows/smoke-test-node16.yml

@@ -8,9 +8,9 @@ jobs:
       - uses: actions/checkout@v2
         with:
           fetch-depth: 0
-      - uses: actions/setup-node@v2
+      - uses: actions/setup-node@v3
         with:
-          node-version: '16'
+          node-version: 16
       - run: yarn
       - run: yarn build
       - uses: ./

.github/workflows/smoke-test-node18.yml

@@ -8,9 +8,9 @@ jobs:
       - uses: actions/checkout@v2
         with:
           fetch-depth: 0
-      - uses: actions/setup-node@v2
+      - uses: actions/setup-node@v3
         with:
-          node-version: '18'
+          node-version: 18
       - run: yarn
       - run: yarn build
       - uses: ./

.github/workflows/smoke-test-node20.yml

@@ -8,9 +8,9 @@ jobs:
       - uses: actions/checkout@v2
         with:
           fetch-depth: 0
-      - uses: actions/setup-node@v2
+      - uses: actions/setup-node@v3
         with:
-          node-version: '20'
+          node-version: 20
       - run: yarn
       - run: yarn build
       - uses: ./

.github/workflows/smoke-test-npx.yml

@@ -5,12 +5,12 @@ jobs:
   chromatic:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/setup-node@v1
-        with:
-          node-version: '16.x'
       - uses: actions/checkout@v2
         with:
           fetch-depth: 0
+      - uses: actions/setup-node@v3
+        with:
+          node-version: 16
       - name: install
         run: yarn && git status --porcelain
       - name: prep package

.github/workflows/smoke-test-windows.yml

@@ -5,12 +5,12 @@ jobs:
   chromatic:
     runs-on: windows-latest
     steps:
-      - uses: actions/setup-node@v1
-        with:
-          node-version: '16.x'
       - uses: actions/checkout@v2
         with:
           fetch-depth: 0
+      - uses: actions/setup-node@v3
+        with:
+          node-version: 16
       - name: install
         run: yarn
       - name: prep package

.github/workflows/smoke-test-yarn-berry.yml

@@ -10,9 +10,13 @@ jobs:
           fetch-depth: 0
       - uses: actions/setup-node@v3
         with:
-            node-version: '16'
-      - run: yarn install
+          node-version: 18
+      - run: rm -rf subdir # remove conflicting subproject
       - run: yarn set version berry
+      - name: install
+        run: yarn install
+        env:
+          YARN_ENABLE_IMMUTABLE_INSTALLS: false
       - name: run chromatic
         run: npx -p . chromatic --build-script-name build-test-storybook --exit-zero-on-changes --force-rebuild
         env:

.github/workflows/smoke-test-yarn-canary.yml

@@ -5,15 +5,18 @@ jobs:
   chromatic:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/setup-node@v1
-        with:
-          node-version: '18.x'
       - uses: actions/checkout@v2
         with:
           fetch-depth: 0
-      - run: yarn install
-      - run: yarn set version berry
+      - uses: actions/setup-node@v3
+        with:
+          node-version: 18
+      - run: rm -rf subdir # remove conflicting subproject
       - run: yarn set version canary
+      - name: install
+        run: yarn install
+        env:
+          YARN_ENABLE_IMMUTABLE_INSTALLS: false
       - name: run chromatic
         run: npx -p . chromatic --build-script-name build-test-storybook --exit-zero-on-changes --force-rebuild
         env:

.github/workflows/smoke-test-yarn-classic.yml

@@ -5,14 +5,15 @@ jobs:
   chromatic:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/setup-node@v1
-        with:
-          node-version: '16.x'
       - uses: actions/checkout@v2
         with:
           fetch-depth: 0
-      - run: yarn install
+      - uses: actions/setup-node@v3
+        with:
+          node-version: 16
+      - run: rm -rf subdir # remove conflicting subproject
       - run: yarn set version 1.x.x
+      - run: yarn install
       - name: run chromatic
         run: npx -p . chromatic --build-script-name build-test-storybook --exit-zero-on-changes --force-rebuild
         env:

node-src/lib/getPackageManager.ts

@@ -0,0 +1,11 @@
+import { parseNr, getCliCommand, parseNa } from '@antfu/ni';
+
+// 'npm' | 'pnpm' | 'yarn' | 'bun'
+export const getPackageManagerName = async () => {
+  return getCliCommand(parseNa, [], { programmatic: true }) as any;
+};
+
+// e.g. `npm run build-storybook`
+export const getPackageManagerRunCommand = async (args: string[]) => {
+  return getCliCommand(parseNr, args, { programmatic: true });
+};

node-src/main.test.ts

@@ -146,7 +146,7 @@ jest.mock('node-fetch', () =>
       if (query.match('SnapshotBuildQuery')) {
         return {
           data: {
-            app: { build: { status: 'PENDING', changeCount: 1, completedAt: 1 } },
+            app: { build: { status: 'PENDING', changeCount: 1, completedAt: 12345 } },
           },
         };
       }
@@ -170,6 +170,7 @@ jest.mock('node-fetch', () =>
                   status: 'PASSED',
                   commit: 'baseline',
                   committedAt: 1234,
+                  completedAt: 12345,
                   changeCount: 1,
                 },
               ],
@@ -275,6 +276,13 @@ jest.mock('./git/getParentCommits', () => ({
 
 const getCommit = <jest.MockedFunction<typeof git.getCommit>>git.getCommit;
 
+jest.mock('./lib/emailHash');
+
+jest.mock('./lib/getPackageManager', () => ({
+  getPackageManagerName: () => Promise.resolve('pnpm'),
+  getPackageManagerRunCommand: (args) => Promise.resolve(`pnpm run ${args.join(' ')}`),
+}));
+
 jest.mock('./lib/getStorybookInfo', () => () => ({
   version: '5.1.0',
   viewLayer: 'viewLayer',

node-src/tasks/build.test.ts

@@ -1,10 +1,16 @@
 import execaDefault from 'execa';
+import mockfs from 'mock-fs';
 
 import { buildStorybook, setSourceDir, setSpawnParams } from './build';
 
 jest.mock('execa');
 
 const execa = <jest.MockedFunction<typeof execaDefault>>execaDefault;
+const execaCommand = <jest.MockedFunction<typeof execaDefault.command>>execaDefault.command;
+
+afterEach(() => {
+  mockfs.restore();
+});
 
 describe('setSourceDir', () => {
   it('sets a random temp directory path on the context', async () => {
@@ -38,57 +44,71 @@ describe('setSpawnParams', () => {
   beforeEach(() => {
     process.env.npm_execpath = npmExecPath;
     execa.mockReturnValue(Promise.resolve({ stdout: '1.2.3' }) as any);
+    execaCommand.mockReturnValue(Promise.resolve({ stdout: '1.2.3' }) as any);
   });
 
   it('sets the spawn params on the context', async () => {
-    process.env.npm_execpath = 'npm';
+    mockfs({ './package.json': JSON.stringify({ packageManager: 'npm' }) });
+
     const ctx = {
       sourceDir: './source-dir/',
       options: { buildScriptName: 'build:storybook' },
       storybook: { version: '6.2.0' },
       git: { changedFiles: ['./index.js'] },
     } as any;
     await setSpawnParams(ctx);
+
     expect(ctx.spawnParams).toEqual({
       client: 'npm',
       clientVersion: '1.2.3',
       nodeVersion: '1.2.3',
       platform: expect.stringMatching(/darwin|linux|win32/),
-      command: 'npm',
-      clientArgs: ['run', '--silent'],
-      scriptArgs: [
-        'build:storybook',
-        '--',
-        '--output-dir',
-        './source-dir/',
-        '--webpack-stats-json',
-        './source-dir/',
-      ],
+      command:
+        'npm run build:storybook -- --output-dir ./source-dir/ --webpack-stats-json ./source-dir/',
     });
   });
 
   it('supports yarn', async () => {
-    process.env.npm_execpath = '/path/to/yarn.js';
+    mockfs({ './package.json': JSON.stringify({ packageManager: 'yarn' }) });
+
     const ctx = {
       sourceDir: './source-dir/',
       options: { buildScriptName: 'build:storybook' },
       storybook: { version: '6.1.0' },
       git: {},
     } as any;
     await setSpawnParams(ctx);
+
     expect(ctx.spawnParams).toEqual({
       client: 'yarn',
       clientVersion: '1.2.3',
       nodeVersion: '1.2.3',
       platform: expect.stringMatching(/darwin|linux|win32/),
-      command: expect.stringMatching(/node/),
-      clientArgs: ['/path/to/yarn.js', 'run'],
-      scriptArgs: ['build:storybook', '--output-dir', './source-dir/'],
+      command: 'yarn run build:storybook --output-dir ./source-dir/',
+    });
+  });
+
+  it('supports pnpm', async () => {
+    mockfs({ './package.json': JSON.stringify({ packageManager: 'pnpm' }) });
+
+    const ctx = {
+      sourceDir: './source-dir/',
+      options: { buildScriptName: 'build:storybook' },
+      storybook: { version: '6.1.0' },
+      git: {},
+    } as any;
+    await setSpawnParams(ctx);
+
+    expect(ctx.spawnParams).toEqual({
+      client: 'pnpm',
+      clientVersion: '1.2.3',
+      nodeVersion: '1.2.3',
+      platform: expect.stringMatching(/darwin|linux|win32/),
+      command: 'pnpm run build:storybook --output-dir ./source-dir/',
     });
   });
 
   it('warns if --only-changes is not supported', async () => {
-    process.env.npm_execpath = 'npm';
     const ctx = {
       sourceDir: './source-dir/',
       options: { buildScriptName: 'build:storybook' },
@@ -106,19 +126,14 @@ describe('setSpawnParams', () => {
 describe('buildStorybook', () => {
   it('runs the build command', async () => {
     const ctx = {
-      spawnParams: {
-        command: 'build:storybook',
-        clientArgs: ['--client-args'],
-        scriptArgs: ['--script-args'],
-      },
+      spawnParams: { command: 'npm run build:storybook --script-args' },
       env: { STORYBOOK_BUILD_TIMEOUT: 1000 },
       log: { debug: jest.fn() },
     } as any;
     await buildStorybook(ctx);
     expect(ctx.buildLogFile).toMatch(/build-storybook\.log$/);
-    expect(execa).toHaveBeenCalledWith(
-      'build:storybook',
-      ['--client-args', '--script-args'],
+    expect(execaCommand).toHaveBeenCalledWith(
+      'npm run build:storybook --script-args',
       expect.objectContaining({ stdio: expect.any(Array) })
     );
     expect(ctx.log.debug).toHaveBeenCalledWith(
@@ -129,16 +144,12 @@ describe('buildStorybook', () => {
 
   it('fails when build times out', async () => {
     const ctx = {
-      spawnParams: {
-        command: 'build:storybook',
-        clientArgs: ['--client-args'],
-        scriptArgs: ['--script-args'],
-      },
+      spawnParams: { command: 'npm run build:storybook --script-args' },
       options: { buildScriptName: '' },
       env: { STORYBOOK_BUILD_TIMEOUT: 0 },
       log: { debug: jest.fn(), error: jest.fn() },
     } as any;
-    execa.mockReturnValue(new Promise((resolve) => setTimeout(resolve, 100)) as any);
+    execaCommand.mockReturnValue(new Promise((resolve) => setTimeout(resolve, 100)) as any);
     await expect(buildStorybook(ctx)).rejects.toThrow('Command failed');
     expect(ctx.log.error).toHaveBeenCalledWith(expect.stringContaining('Operation timed out'));
   });

node-src/tasks/build.ts

@@ -10,6 +10,7 @@ import { Context } from '../types';
 import { endActivity, startActivity } from '../ui/components/activity';
 import buildFailed from '../ui/messages/errors/buildFailed';
 import { failed, initial, pending, skipped, success } from '../ui/tasks/build';
+import { getPackageManagerName, getPackageManagerRunCommand } from '../lib/getPackageManager';
 
 const trimOutput = ({ stdout }) => stdout && stdout.toString().trim();
 
@@ -34,36 +35,26 @@ export const setSpawnParams = async (ctx) => {
     ctx.log.warn('Storybook version 6.2.0 or later is required to use the --only-changed flag');
   }
 
-  // Run either:
-  //   node path/to/npm-cli.js run build-storybook
-  //   node path/to/yarn.js run build-storybook
-  //   npm run build-storybook
-  // Based on https://github.com/mysticatea/npm-run-all/blob/52eaf86242ba408dedd015f53ca7ca368f25a026/lib/run-task.js#L156-L174
-  const npmExecPath = process.env.npm_execpath;
-  const npmExecFile = npmExecPath && path.basename(npmExecPath);
-  const isJsPath = npmExecFile && /\.m?js$/.test(npmExecFile);
-  const isYarn = npmExecFile && npmExecFile.includes('yarn');
-  const isNpx = npmExecFile && npmExecFile.includes('npx');
-
-  const client = isYarn ? 'yarn' : 'npm';
+  const client = await getPackageManagerName();
   const clientVersion = await execa(client, ['--version']).then(trimOutput);
   const nodeVersion = await execa('node', ['--version']).then(trimOutput);
 
-  ctx.spawnParams = {
-    client,
-    clientVersion,
-    nodeVersion,
-    platform: process.platform,
-    command: (!isNpx && (isJsPath ? process.execPath : npmExecPath)) || 'npm',
-    clientArgs: !isNpx && isJsPath ? [npmExecPath, 'run'] : ['run', '--silent'],
-    scriptArgs: [
+  const command = await getPackageManagerRunCommand(
+    [
       ctx.options.buildScriptName,
-      isYarn ? '' : '--',
       '--output-dir',
       ctx.sourceDir,
       ctx.git.changedFiles && webpackStatsSupported && '--webpack-stats-json',
       ctx.git.changedFiles && webpackStatsSupported && ctx.sourceDir,
-    ].filter(Boolean),
+    ].filter(Boolean)
+  );
+
+  ctx.spawnParams = {
+    client,
+    clientVersion,
+    nodeVersion,
+    platform: process.platform,
+    command,
   };
 };
 
@@ -79,10 +70,10 @@ export const buildStorybook = async (ctx: Context) => {
   });
 
   try {
-    const { command, clientArgs, scriptArgs } = ctx.spawnParams;
+    const { command } = ctx.spawnParams;
     ctx.log.debug('Using spawnParams:', JSON.stringify(ctx.spawnParams, null, 2));
     await Promise.race([
-      execa(command, [...clientArgs, ...scriptArgs], { stdio: [null, logFile, logFile] }),
+      execa.command(command, { stdio: [null, logFile, logFile] }),
       timeoutAfter(ctx.env.STORYBOOK_BUILD_TIMEOUT),
     ]);
   } catch (e) {