From 369707e23b6d3d0dc932a59609eb49252d2341ae Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Wed, 10 Mar 2021 17:09:54 +1100 Subject: [PATCH 1/4] Fix tests and actually return stats in all cases --- lib/builder-webpack4/src/index.ts | 17 ++++++++++------- lib/core-server/src/core-presets.test.ts | 8 ++++---- lib/core-server/src/manager/builder.ts | 20 ++++++++++++++++---- 3 files changed, 30 insertions(+), 15 deletions(-) diff --git a/lib/builder-webpack4/src/index.ts b/lib/builder-webpack4/src/index.ts index c476800c0e84..848b935b9946 100644 --- a/lib/builder-webpack4/src/index.ts +++ b/lib/builder-webpack4/src/index.ts @@ -39,6 +39,13 @@ export const executor = { get: webpack, }; +export const makeStatsFromError = (err: string) => + (({ + hasErrors: () => true, + hasWarnings: () => false, + toJson: () => ({ warnings: [] as any[], errors: [err] }), + } as any) as Stats); + export const start: WebpackBuilder['start'] = async ({ startTime, options, router }) => { const config = await getConfig(options); const compiler = executor.get(config); @@ -48,11 +55,7 @@ export const start: WebpackBuilder['start'] = async ({ startTime, options, route return { bail, totalTime: process.hrtime(startTime), - stats: ({ - hasErrors: () => true, - hasWarngins: () => false, - toJson: () => ({ warnings: [] as any[], errors: [err] }), - } as any) as Stats, + stats: makeStatsFromError(err), }; } @@ -117,10 +120,10 @@ export const build: WebpackBuilder['build'] = async ({ options, startTime }) => if (!compiler) { const err = `${config.name}: missing webpack compiler at runtime!`; logger.error(err); - return; + return Promise.resolve(makeStatsFromError(err)); } - await new Promise((succeed, fail) => { + return new Promise((succeed, fail) => { compiler.run((error, stats) => { if (error || !stats || stats.hasErrors()) { logger.error('=> Failed to build the preview'); diff --git a/lib/core-server/src/core-presets.test.ts b/lib/core-server/src/core-presets.test.ts index 4df9a4ca871a..d4918fdf989f 100644 --- a/lib/core-server/src/core-presets.test.ts +++ b/lib/core-server/src/core-presets.test.ts @@ -193,8 +193,8 @@ describe('dev cli flags', () => { await buildDevStandalone({ ...cliOptions, webpackStatsJson: '/tmp/dir' }); expect(outputStats).toHaveBeenCalledWith( '/tmp/dir', - expect.objectContaining({}), - expect.objectContaining({}) + expect.objectContaining({ toJson: expect.any(Function) }), + expect.objectContaining({ toJson: expect.any(Function) }) ); }); @@ -233,8 +233,8 @@ describe('build cli flags', () => { await buildStaticStandalone({ ...cliOptions, webpackStatsJson: '/tmp/dir' }); expect(outputStats).toHaveBeenCalledWith( '/tmp/dir', - expect.objectContaining({}), - expect.objectContaining({}) + expect.objectContaining({ toJson: expect.any(Function) }), + expect.objectContaining({ toJson: expect.any(Function) }) ); }); }); diff --git a/lib/core-server/src/manager/builder.ts b/lib/core-server/src/manager/builder.ts index 507a6b8eeed2..9cab145a172c 100644 --- a/lib/core-server/src/manager/builder.ts +++ b/lib/core-server/src/manager/builder.ts @@ -19,6 +19,13 @@ export const executor = { get: webpack, }; +export const makeStatsFromError = (err: string) => + (({ + hasErrors: () => true, + hasWarnings: () => false, + toJson: () => ({ warnings: [] as any[], errors: [err] }), + } as any) as Stats); + export const start: WebpackBuilder['start'] = async ({ startTime, options, router }) => { const prebuiltDir = await getPrebuiltDir(options); const config = await getConfig(options); @@ -47,7 +54,12 @@ export const start: WebpackBuilder['start'] = async ({ startTime, options, route if (!compiler) { const err = `${config.name}: missing webpack compiler at runtime!`; logger.error(err); - return; + // eslint-disable-next-line consistent-return + return { + bail, + totalTime: process.hrtime(startTime), + stats: makeStatsFromError(err), + }; } const { handler, modulesCount } = await useProgressReporting(router, startTime, options); @@ -103,10 +115,10 @@ export const build: WebpackBuilder['build'] = async ({ options, startTime }) => if (!compiler) { const err = `${config.name}: missing webpack compiler at runtime!`; logger.error(err); - return; + return Promise.resolve(makeStatsFromError(err)); } - await new Promise((succeed, fail) => { + return new Promise((succeed, fail) => { compiler.run((error, stats) => { if (error || !stats || stats.hasErrors()) { logger.error('=> Failed to build the manager'); @@ -131,7 +143,7 @@ export const build: WebpackBuilder['build'] = async ({ options, startTime }) => ); statsData?.warnings?.forEach((e) => logger.warn(e)); - succeed(); + succeed(stats); } }); }); From 267d001e3b2eeaec047d5a6ce12fc303d9e75a2a Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Thu, 11 Mar 2021 14:30:21 +1100 Subject: [PATCH 2/4] Small tweak --- lib/builder-webpack4/src/index.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/builder-webpack4/src/index.ts b/lib/builder-webpack4/src/index.ts index 848b935b9946..de78a2803471 100644 --- a/lib/builder-webpack4/src/index.ts +++ b/lib/builder-webpack4/src/index.ts @@ -39,12 +39,12 @@ export const executor = { get: webpack, }; -export const makeStatsFromError = (err: string) => - (({ +export const makeStatsFromError: (err: string) => Stats = (err) => + ({ hasErrors: () => true, hasWarnings: () => false, toJson: () => ({ warnings: [] as any[], errors: [err] }), - } as any) as Stats); + } as any); export const start: WebpackBuilder['start'] = async ({ startTime, options, router }) => { const config = await getConfig(options); From 595921950ed6a4b03858873700ba0425033e8180 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Thu, 11 Mar 2021 15:37:36 +1100 Subject: [PATCH 3/4] Don't default the directory for `--webpack-stats-json` --- docs/api/cli-options.md | 4 ++-- examples/official-storybook/package.json | 2 +- lib/core-server/src/cli/dev.ts | 6 +----- lib/core-server/src/cli/prod.ts | 6 +----- 4 files changed, 5 insertions(+), 13 deletions(-) diff --git a/docs/api/cli-options.md b/docs/api/cli-options.md index ecf020ca6520..81278e787626 100644 --- a/docs/api/cli-options.md +++ b/docs/api/cli-options.md @@ -29,7 +29,7 @@ Usage: start-storybook [options] | --quiet | Suppress verbose build output | `start-storybook --quiet` | | --no-dll | Do not use dll reference (no-op) | `start-storybook --no-dll` | | --debug-webpack | Display final webpack configurations for debugging purposes | `start-storybook --debug-webpack` | -| `--webpack-stats-json [directory]` | Write Webpack Stats JSON to disk | `start-storybook --webpack-stats-json /tmp/webpack-stats` | +| `--webpack-stats-json ` | Write Webpack Stats JSON to disk | `start-storybook --webpack-stats-json /tmp/webpack-stats` | | --docs | Starts Storybook in documentation mode. Learn more about it in [here](../writing-docs/build-documentation.md#preview-storybooks-documentation) | `start-storybook --docs` | | --no-manager-cache | Disables Storybook's manager caching mechanism. See note below. | `start-storybook --no-manager-cache` | @@ -55,7 +55,7 @@ Usage: build-storybook [options] | --quiet | Suppress verbose build output | `build-storybook --quiet` | | --no-dll | Do not use dll reference (no-op) | `build-storybook --no-dll` | | --debug-webpack | Display final webpack configurations for debugging purposes | `build-storybook --debug-webpack` | -| `--webpack-stats-json [directory]` | Write Webpack Stats JSON to disk | `start-storybook --webpack-stats-json /tmp/webpack-stats` | +| `--webpack-stats-json ` | Write Webpack Stats JSON to disk | `start-storybook --webpack-stats-json /tmp/webpack-stats` | | --docs | Builds Storybook in documentation mode. Learn more about it in [here](../writing-docs/build-documentation.md#publish-storybooks-documentation)) | `build-storybook --docs` |
diff --git a/examples/official-storybook/package.json b/examples/official-storybook/package.json index 98c95f00e4b4..796e4f5821eb 100644 --- a/examples/official-storybook/package.json +++ b/examples/official-storybook/package.json @@ -8,7 +8,7 @@ "do-storyshots-puppeteer": "../../node_modules/.bin/jest --projects=./storyshots-puppeteer", "generate-addon-jest-testresults": "jest --config=tests/addon-jest.config.json --json --outputFile=stories/addon-jest.testresults.json", "graphql": "node ./graphql-server/index.js", - "packtracker": "yarn storybook --smoke-test --webpack-stats-json --quiet && cross-env PT_PROJECT_TOKEN=1af1d41b-d737-41d4-ac00-53c8f3913b53 packtracker-upload --stats=./node_modules/.cache/storybook/public/manager-stats.json", + "packtracker": "yarn storybook --smoke-test --webpack-stats-json /tmp --quiet && cross-env PT_PROJECT_TOKEN=1af1d41b-d737-41d4-ac00-53c8f3913b53 packtracker-upload --stats=/tmp/manager-stats.json", "storybook": "cross-env STORYBOOK_DISPLAY_WARNING=true DISPLAY_WARNING=true start-storybook -p 9011 -c ./", "storyshots-puppeteer": "yarn run build-storybook && yarn run do-storyshots-puppeteer" }, diff --git a/lib/core-server/src/cli/dev.ts b/lib/core-server/src/cli/dev.ts index f54eb949c8fe..90af35ba0210 100644 --- a/lib/core-server/src/cli/dev.ts +++ b/lib/core-server/src/cli/dev.ts @@ -42,11 +42,7 @@ export async function getDevCli(packageJson: { .option('--docs-dll', 'Use Docs dll reference (legacy)') .option('--ui-dll', 'Use UI dll reference (legacy)') .option('--debug-webpack', 'Display final webpack configurations for debugging purposes') - .option( - '--webpack-stats-json [directory]', - 'Write Webpack Stats JSON to disk', - resolvePathInStorybookCache(`public/`) - ) + .option('--webpack-stats-json [directory]', 'Write Webpack Stats JSON to disk') .option( '--preview-url [string]', 'Disables the default storybook preview and lets your use your own' diff --git a/lib/core-server/src/cli/prod.ts b/lib/core-server/src/cli/prod.ts index 569d6f6a7ad7..7f75e0f4ddaa 100644 --- a/lib/core-server/src/cli/prod.ts +++ b/lib/core-server/src/cli/prod.ts @@ -37,11 +37,7 @@ export function getProdCli(packageJson: { .option('--docs-dll', 'Use Docs dll reference (legacy)') .option('--ui-dll', 'Use UI dll reference (legacy)') .option('--debug-webpack', 'Display final webpack configurations for debugging purposes') - .option( - '--webpack-stats-json [directory]', - 'Write Webpack Stats JSON to disk', - resolvePathInStorybookCache(`public/`) - ) + .option('--webpack-stats-json [directory]', 'Write Webpack Stats JSON to disk') .option( '--preview-url [string]', 'Disables the default storybook preview and lets your use your own' From 09fc7ebdcd1275460c54c7050b399c149a28dcc1 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Thu, 11 Mar 2021 15:39:44 +1100 Subject: [PATCH 4/4] Linting --- lib/core-server/src/cli/dev.ts | 2 +- lib/core-server/src/cli/prod.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/core-server/src/cli/dev.ts b/lib/core-server/src/cli/dev.ts index 90af35ba0210..c0f7a60b27e0 100644 --- a/lib/core-server/src/cli/dev.ts +++ b/lib/core-server/src/cli/dev.ts @@ -1,7 +1,7 @@ import program, { CommanderStatic } from 'commander'; import chalk from 'chalk'; import { logger } from '@storybook/node-logger'; -import { CLIOptions, resolvePathInStorybookCache } from '@storybook/core-common'; +import { CLIOptions } from '@storybook/core-common'; import { parseList, getEnvConfig, checkDeprecatedFlags } from './utils'; export async function getDevCli(packageJson: { diff --git a/lib/core-server/src/cli/prod.ts b/lib/core-server/src/cli/prod.ts index 7f75e0f4ddaa..205032b8473b 100644 --- a/lib/core-server/src/cli/prod.ts +++ b/lib/core-server/src/cli/prod.ts @@ -1,7 +1,6 @@ import program, { CommanderStatic } from 'commander'; import chalk from 'chalk'; import { logger } from '@storybook/node-logger'; -import { resolvePathInStorybookCache } from '@storybook/core-common'; import { parseList, getEnvConfig, checkDeprecatedFlags } from './utils'; export interface ProdCliOptions {