From e76c9c2be18586a2e0d179b671dca1e8aeee97ef Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Thu, 11 Mar 2021 11:54:13 +0100 Subject: [PATCH 01/15] Report the client we're using to run npm scripts with --- bin/tasks/build.js | 3 +++ bin/ui/tasks/build.js | 4 ++-- bin/ui/tasks/build.stories.js | 14 +++++++++----- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/bin/tasks/build.js b/bin/tasks/build.js index fcda2fd49..e2ee4bb32 100644 --- a/bin/tasks/build.js +++ b/bin/tasks/build.js @@ -29,6 +29,8 @@ export const setSpawnParams = (ctx) => { const isJsPath = typeof npmExecPath === 'string' && /\.m?js/.test(path.extname(npmExecPath)); const isYarn = npmExecPath && path.basename(npmExecPath) === 'yarn.js'; ctx.spawnParams = { + client: isYarn ? 'yarn' : 'npm', + platform: process.platform, command: (isJsPath ? process.execPath : npmExecPath) || 'npm', clientArgs: isJsPath ? [npmExecPath, 'run'] : ['run', '--silent'], scriptArgs: [ @@ -53,6 +55,7 @@ export const buildStorybook = async (ctx) => { try { const { command, clientArgs, scriptArgs } = ctx.spawnParams; + ctx.log.debug('Using spawnParams:', JSON.stringify(ctx.spawnParams, null, 2)); await Promise.race([ execa(command, [...clientArgs, ...scriptArgs], { stdio: [null, logFile, logFile] }), timeoutAfter(ctx.env.STORYBOOK_BUILD_TIMEOUT), diff --git a/bin/ui/tasks/build.js b/bin/ui/tasks/build.js index ecc452099..dd5d6ea02 100644 --- a/bin/ui/tasks/build.js +++ b/bin/ui/tasks/build.js @@ -7,7 +7,7 @@ export const initial = { export const pending = (ctx) => ({ status: 'pending', - title: 'Building your Storybook', + title: `Building your Storybook with ${ctx.spawnParams.client}`, output: `Running command: ${ctx.spawnParams.scriptArgs.join(' ')}`, }); @@ -25,6 +25,6 @@ export const skipped = (ctx) => ({ export const failed = (ctx) => ({ status: 'error', - title: 'Building your Storybook', + title: `Building your Storybook with ${ctx.spawnParams.client}`, output: `Command failed: ${ctx.spawnParams.scriptArgs.join(' ')}`, }); diff --git a/bin/ui/tasks/build.stories.js b/bin/ui/tasks/build.stories.js index 465fb1ab2..122efe5ab 100644 --- a/bin/ui/tasks/build.stories.js +++ b/bin/ui/tasks/build.stories.js @@ -1,17 +1,19 @@ import task from '../components/task'; -import { initial, pending, skipped, success } from './build'; +import { initial, pending, skipped, success, failed } from './build'; export default { title: 'CLI/Tasks/Build', decorators: [(storyFn) => task(storyFn())], }; +const spawnParams = { + client: 'yarn', + scriptArgs: ['build-storybook', '-o', 'storybook-static'], +}; + export const Initial = () => initial; -export const Building = () => - pending({ - spawnParams: { scriptArgs: ['build-storybook', '-o', 'storybook-static'] }, - }); +export const Building = () => pending({ spawnParams }); export const Built = () => success({ @@ -24,3 +26,5 @@ export const Skipped = () => skipped({ options: { storybookBuildDir: '/users/me/project/storybook-static' }, }); + +export const Failed = () => failed({ spawnParams }); From cc1ef62c71e430b01ad0e21ce4292dacce0a116a Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Thu, 11 Mar 2021 12:06:57 +0100 Subject: [PATCH 02/15] Prefer locally installed packages from node_modules/.bin and log spawnParams on fatal error --- bin/tasks/build.js | 11 +++++++++-- bin/ui/messages/errors/fatalError.js | 3 ++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/bin/tasks/build.js b/bin/tasks/build.js index e2ee4bb32..5fe307d54 100644 --- a/bin/tasks/build.js +++ b/bin/tasks/build.js @@ -39,6 +39,10 @@ export const setSpawnParams = (ctx) => { '--output-dir', ctx.sourceDir, ].filter(Boolean), + spawnOptions: { + preferLocal: true, + localDir: path.resolve('node_modules/.bin'), + }, }; }; @@ -54,10 +58,13 @@ export const buildStorybook = async (ctx) => { }); try { - const { command, clientArgs, scriptArgs } = ctx.spawnParams; + const { command, clientArgs, scriptArgs, spawnOptions } = 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, [...clientArgs, ...scriptArgs], { + stdio: [null, logFile, logFile], + ...spawnOptions, + }), timeoutAfter(ctx.env.STORYBOOK_BUILD_TIMEOUT), ]); } catch (e) { diff --git a/bin/ui/messages/errors/fatalError.js b/bin/ui/messages/errors/fatalError.js index a1e62badc..3cc33a777 100644 --- a/bin/ui/messages/errors/fatalError.js +++ b/bin/ui/messages/errors/fatalError.js @@ -14,7 +14,7 @@ export default function fatalError(ctx, error, timestamp = new Date().toISOStrin const website = link(pkg.docs); const errors = [].concat(error); - const { git = {}, storybook, exitCode, isolatorUrl, cachedUrl, build } = ctx; + const { git = {}, storybook, spawnParams, exitCode, isolatorUrl, cachedUrl, build } = ctx; const debugInfo = { timestamp, sessionId, @@ -27,6 +27,7 @@ export default function fatalError(ctx, error, timestamp = new Date().toISOStrin flags, ...(options.buildScriptName ? { buildScript: scripts[options.buildScriptName] } : {}), ...(options.scriptName ? { storybookScript: scripts[options.scriptName] } : {}), + ...(spawnParams ? { spawnParams } : {}), exitCode, errorType: errors.map((err) => err.name).join('\n'), errorMessage: stripAnsi(errors[0].message.split('\n')[0].trim()), From d536c104584c15d6d14af771fef4ba7d0d22b7d8 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Thu, 11 Mar 2021 12:58:26 +0100 Subject: [PATCH 03/15] Update tests --- bin/tasks/build.test.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/bin/tasks/build.test.js b/bin/tasks/build.test.js index ad98b77ea..d7bdc74ee 100644 --- a/bin/tasks/build.test.js +++ b/bin/tasks/build.test.js @@ -36,9 +36,15 @@ describe('setSpawnParams', () => { const ctx = { sourceDir: './source-dir/', options: { buildScriptName: 'build:storybook' } }; await setSpawnParams(ctx); expect(ctx.spawnParams).toEqual({ + client: 'npm', + platform: expect.stringMatching(/darwin|linux|win32/), command: 'npm', clientArgs: ['run', '--silent'], scriptArgs: ['build:storybook', '--', '--output-dir', './source-dir/'], + spawnOptions: { + preferLocal: true, + localDir: expect.stringMatching(/node_modules[/\\]\.bin$/), + }, }); }); }); @@ -52,6 +58,7 @@ describe('buildStorybook', () => { scriptArgs: ['--script-args'], }, env: { STORYBOOK_BUILD_TIMEOUT: 1000 }, + log: { debug: jest.fn() }, }; await buildStorybook(ctx); expect(ctx.buildLogFile).toMatch(/build-storybook\.log$/); @@ -60,6 +67,10 @@ describe('buildStorybook', () => { ['--client-args', '--script-args'], expect.objectContaining({ stdio: expect.any(Array) }) ); + expect(ctx.log.debug).toHaveBeenCalledWith( + 'Using spawnParams:', + JSON.stringify(ctx.spawnParams, null, 2) + ); }); it('fails when build times out', async () => { @@ -71,7 +82,7 @@ describe('buildStorybook', () => { }, options: { buildScriptName: '' }, env: { STORYBOOK_BUILD_TIMEOUT: 0 }, - log: { error: jest.fn() }, + log: { debug: jest.fn(), error: jest.fn() }, }; execa.mockReturnValue(new Promise((resolve) => setTimeout(resolve, 10))); await expect(buildStorybook(ctx)).rejects.toThrow('Command failed'); From 53705007e224497ea0c2eda04abe3bf819338971 Mon Sep 17 00:00:00 2001 From: Adrian Macneil Date: Wed, 3 Mar 2021 17:24:54 -0800 Subject: [PATCH 04/15] Support yarn execpath --- bin/tasks/build.js | 2 +- bin/tasks/build.test.js | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/bin/tasks/build.js b/bin/tasks/build.js index 5fe307d54..0691c2f3e 100644 --- a/bin/tasks/build.js +++ b/bin/tasks/build.js @@ -27,7 +27,7 @@ export const setSpawnParams = (ctx) => { // Based on https://github.com/mysticatea/npm-run-all/blob/52eaf86242ba408dedd015f53ca7ca368f25a026/lib/run-task.js#L156-L174 const npmExecPath = process.env.npm_execpath; const isJsPath = typeof npmExecPath === 'string' && /\.m?js/.test(path.extname(npmExecPath)); - const isYarn = npmExecPath && path.basename(npmExecPath) === 'yarn.js'; + const isYarn = npmExecPath && /^yarn(\.js)?$/.test(path.basename(npmExecPath)); ctx.spawnParams = { client: isYarn ? 'yarn' : 'npm', platform: process.platform, diff --git a/bin/tasks/build.test.js b/bin/tasks/build.test.js index d7bdc74ee..a34d0965a 100644 --- a/bin/tasks/build.test.js +++ b/bin/tasks/build.test.js @@ -47,6 +47,17 @@ describe('setSpawnParams', () => { }, }); }); + + it('supports yarn', async () => { + process.env.npm_execpath = '/path/to/yarn'; + const ctx = { sourceDir: './source-dir/', options: { buildScriptName: 'build:storybook' } }; + await setSpawnParams(ctx); + expect(ctx.spawnParams).toEqual({ + command: '/path/to/yarn', + clientArgs: ['run', '--silent'], + scriptArgs: ['build:storybook', '--output-dir', './source-dir/'], + }); + }); }); describe('buildStorybook', () => { From 8e3d4bbf24e1e12e30cb12178ad9a689b53ac600 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Thu, 11 Mar 2021 13:15:38 +0100 Subject: [PATCH 05/15] Fix test --- bin/tasks/build.test.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/bin/tasks/build.test.js b/bin/tasks/build.test.js index a34d0965a..53033be9d 100644 --- a/bin/tasks/build.test.js +++ b/bin/tasks/build.test.js @@ -53,9 +53,15 @@ describe('setSpawnParams', () => { const ctx = { sourceDir: './source-dir/', options: { buildScriptName: 'build:storybook' } }; await setSpawnParams(ctx); expect(ctx.spawnParams).toEqual({ + client: 'yarn', + platform: expect.stringMatching(/darwin|linux|win32/), command: '/path/to/yarn', clientArgs: ['run', '--silent'], scriptArgs: ['build:storybook', '--output-dir', './source-dir/'], + spawnOptions: { + preferLocal: true, + localDir: expect.stringMatching(/node_modules[/\\]\.bin$/), + }, }); }); }); From 46065c4376133176df5416fdd33b611168bc7e2f Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Thu, 11 Mar 2021 14:53:27 +0100 Subject: [PATCH 06/15] 5.8.0-canary.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 29308c97e..4bf7c46c8 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "chromatic", - "version": "5.7.0", + "version": "5.8.0-canary.0", "description": "Visual Testing for Storybook", "homepage": "https://www.chromatic.com", "bugs": { From 5df3c4be5c28d40bdde67780683282bbfb88ec98 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Wed, 28 Apr 2021 20:15:09 +0200 Subject: [PATCH 07/15] Extra logging --- bin/tasks/build.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/bin/tasks/build.js b/bin/tasks/build.js index 0691c2f3e..d04345538 100644 --- a/bin/tasks/build.js +++ b/bin/tasks/build.js @@ -50,11 +50,20 @@ const timeoutAfter = (ms) => new Promise((resolve, reject) => setTimeout(reject, ms, new Error(`Operation timed out`))); export const buildStorybook = async (ctx) => { + ctx.log.debug('buildStorybook') ctx.buildLogFile = path.resolve('./build-storybook.log'); + ctx.log.debug(ctx.buildLogFile); const logFile = fs.createWriteStream(ctx.buildLogFile); + ctx.log.debug(logFile); await new Promise((resolve, reject) => { - logFile.on('open', resolve); - logFile.on('error', reject); + logFile.on('open', () => { + ctx.log.debug('open'); + resolve(); + }); + logFile.on('error', (e) => { + ctx.log.debug('error', e); + reject(); + }); }); try { From 39906da6283a8a99757ab8d3841a78ff0a720a07 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Wed, 28 Apr 2021 20:15:32 +0200 Subject: [PATCH 08/15] 5.8.0-debug.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c63e9b0d8..714215ca6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "chromatic", - "version": "5.8.0-canary.0", + "version": "5.8.0-debug.0", "description": "Automate visual testing across browsers. Gather UI feedback. Versioned documentation.", "homepage": "https://www.chromatic.com", "bugs": { From c89e63782bc5825492afedae4237580f04f2b238 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Wed, 28 Apr 2021 20:23:24 +0200 Subject: [PATCH 09/15] Revert "Extra logging" This reverts commit 5df3c4be5c28d40bdde67780683282bbfb88ec98. --- bin/tasks/build.js | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/bin/tasks/build.js b/bin/tasks/build.js index d04345538..0691c2f3e 100644 --- a/bin/tasks/build.js +++ b/bin/tasks/build.js @@ -50,20 +50,11 @@ const timeoutAfter = (ms) => new Promise((resolve, reject) => setTimeout(reject, ms, new Error(`Operation timed out`))); export const buildStorybook = async (ctx) => { - ctx.log.debug('buildStorybook') ctx.buildLogFile = path.resolve('./build-storybook.log'); - ctx.log.debug(ctx.buildLogFile); const logFile = fs.createWriteStream(ctx.buildLogFile); - ctx.log.debug(logFile); await new Promise((resolve, reject) => { - logFile.on('open', () => { - ctx.log.debug('open'); - resolve(); - }); - logFile.on('error', (e) => { - ctx.log.debug('error', e); - reject(); - }); + logFile.on('open', resolve); + logFile.on('error', reject); }); try { From 65fa50e10302b204c02b9cd0b92c159795ab744e Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Wed, 28 Apr 2021 21:08:07 +0200 Subject: [PATCH 10/15] Logging --- bin/tasks/build.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bin/tasks/build.js b/bin/tasks/build.js index 0691c2f3e..7c94b1b9f 100644 --- a/bin/tasks/build.js +++ b/bin/tasks/build.js @@ -28,6 +28,8 @@ export const setSpawnParams = (ctx) => { const npmExecPath = process.env.npm_execpath; const isJsPath = typeof npmExecPath === 'string' && /\.m?js/.test(path.extname(npmExecPath)); const isYarn = npmExecPath && /^yarn(\.js)?$/.test(path.basename(npmExecPath)); + ctx.log.debug('process.env.npm_execpath', process.env.npm_execpath); + ctx.log.debug('process.execPath', process.execPath); ctx.spawnParams = { client: isYarn ? 'yarn' : 'npm', platform: process.platform, From e7ab454cc251d90664fb365083535dbbf9c6b229 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Wed, 28 Apr 2021 21:08:13 +0200 Subject: [PATCH 11/15] 5.8.0-debug.1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 714215ca6..254da0163 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "chromatic", - "version": "5.8.0-debug.0", + "version": "5.8.0-debug.1", "description": "Automate visual testing across browsers. Gather UI feedback. Versioned documentation.", "homepage": "https://www.chromatic.com", "bugs": { From 39db0bc9e4eba988fab6ddbfc35a369be34101df Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Wed, 28 Apr 2021 21:37:33 +0200 Subject: [PATCH 12/15] Revert "Logging" This reverts commit 65fa50e10302b204c02b9cd0b92c159795ab744e. --- bin/tasks/build.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/bin/tasks/build.js b/bin/tasks/build.js index 7c94b1b9f..0691c2f3e 100644 --- a/bin/tasks/build.js +++ b/bin/tasks/build.js @@ -28,8 +28,6 @@ export const setSpawnParams = (ctx) => { const npmExecPath = process.env.npm_execpath; const isJsPath = typeof npmExecPath === 'string' && /\.m?js/.test(path.extname(npmExecPath)); const isYarn = npmExecPath && /^yarn(\.js)?$/.test(path.basename(npmExecPath)); - ctx.log.debug('process.env.npm_execpath', process.env.npm_execpath); - ctx.log.debug('process.execPath', process.execPath); ctx.spawnParams = { client: isYarn ? 'yarn' : 'npm', platform: process.platform, From 5d16dd69f44ac4ee3e38cae0fa293ebac0809ed7 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Wed, 28 Apr 2021 21:47:29 +0200 Subject: [PATCH 13/15] Use yarn-or-npm to determine spawn params --- bin/tasks/build.js | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/bin/tasks/build.js b/bin/tasks/build.js index 0691c2f3e..c3e653467 100644 --- a/bin/tasks/build.js +++ b/bin/tasks/build.js @@ -3,6 +3,7 @@ import fs from 'fs'; import path from 'path'; import semver from 'semver'; import tmp from 'tmp-promise'; +import yarnOrNpm, { hasYarn } from 'yarn-or-npm'; import { createTask, transitionTo } from '../lib/tasks'; import buildFailed from '../ui/messages/errors/buildFailed'; @@ -21,21 +22,14 @@ export const setSourceDir = async (ctx) => { }; export const setSpawnParams = (ctx) => { - // Run either: - // npm/yarn run scriptName (depending on npm_execpath) - // node path/to/npm.js run scriptName (if npm run via node) - // Based on https://github.com/mysticatea/npm-run-all/blob/52eaf86242ba408dedd015f53ca7ca368f25a026/lib/run-task.js#L156-L174 - const npmExecPath = process.env.npm_execpath; - const isJsPath = typeof npmExecPath === 'string' && /\.m?js/.test(path.extname(npmExecPath)); - const isYarn = npmExecPath && /^yarn(\.js)?$/.test(path.basename(npmExecPath)); ctx.spawnParams = { - client: isYarn ? 'yarn' : 'npm', + client: yarnOrNpm(), platform: process.platform, - command: (isJsPath ? process.execPath : npmExecPath) || 'npm', - clientArgs: isJsPath ? [npmExecPath, 'run'] : ['run', '--silent'], + command: yarnOrNpm(), + clientArgs: ['run', '--silent'], scriptArgs: [ ctx.options.buildScriptName, - isYarn ? '' : '--', + hasYarn() ? '' : '--', '--output-dir', ctx.sourceDir, ].filter(Boolean), From 8f5268d389d035f9ea0f18ea5a748659ff43d1ef Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Wed, 28 Apr 2021 21:47:38 +0200 Subject: [PATCH 14/15] 5.8.0-debug.2 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 254da0163..9bbd1c9c8 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "chromatic", - "version": "5.8.0-debug.1", + "version": "5.8.0-debug.2", "description": "Automate visual testing across browsers. Gather UI feedback. Versioned documentation.", "homepage": "https://www.chromatic.com", "bugs": { From a09a643cb8cd9bf198ab852e4eb64aedce838b3a Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Thu, 29 Apr 2021 08:51:24 +0200 Subject: [PATCH 15/15] Update build message --- bin/ui/tasks/build.js | 11 +++++++---- bin/ui/tasks/build.stories.js | 3 ++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/bin/ui/tasks/build.js b/bin/ui/tasks/build.js index dd5d6ea02..2c6c12064 100644 --- a/bin/ui/tasks/build.js +++ b/bin/ui/tasks/build.js @@ -1,5 +1,8 @@ import { getDuration } from '../../lib/tasks'; +const fullCommand = ({ command, clientArgs, scriptArgs }) => + [command, ...clientArgs, ...scriptArgs].join(' '); + export const initial = { status: 'initial', title: 'Build Storybook', @@ -7,8 +10,8 @@ export const initial = { export const pending = (ctx) => ({ status: 'pending', - title: `Building your Storybook with ${ctx.spawnParams.client}`, - output: `Running command: ${ctx.spawnParams.scriptArgs.join(' ')}`, + title: `Building your Storybook`, + output: `Running command: ${fullCommand(ctx.spawnParams)}`, }); export const success = (ctx) => ({ @@ -25,6 +28,6 @@ export const skipped = (ctx) => ({ export const failed = (ctx) => ({ status: 'error', - title: `Building your Storybook with ${ctx.spawnParams.client}`, - output: `Command failed: ${ctx.spawnParams.scriptArgs.join(' ')}`, + title: `Building your Storybook`, + output: `Command failed: ${fullCommand(ctx.spawnParams)}`, }); diff --git a/bin/ui/tasks/build.stories.js b/bin/ui/tasks/build.stories.js index 122efe5ab..1ad11c03b 100644 --- a/bin/ui/tasks/build.stories.js +++ b/bin/ui/tasks/build.stories.js @@ -7,7 +7,8 @@ export default { }; const spawnParams = { - client: 'yarn', + command: 'yarn', + clientArgs: ['run', '--silent'], scriptArgs: ['build-storybook', '-o', 'storybook-static'], };