From bc8f1b74edb953b908c743ec40f0bab91e770ce2 Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Mon, 19 Feb 2024 15:50:34 +0100 Subject: [PATCH 1/4] remove the logging to file feature from autoblockers --- .../src/autoblock/block-dependencies-versions.ts | 3 --- code/lib/cli/src/autoblock/block-node-version.ts | 3 --- code/lib/cli/src/autoblock/block-stories-mdx.ts | 11 +++++------ code/lib/cli/src/autoblock/block-storystorev6.ts | 5 ----- code/lib/cli/src/autoblock/index.ts | 16 +--------------- code/lib/cli/src/autoblock/types.ts | 7 ------- 6 files changed, 6 insertions(+), 39 deletions(-) diff --git a/code/lib/cli/src/autoblock/block-dependencies-versions.ts b/code/lib/cli/src/autoblock/block-dependencies-versions.ts index 284562aa9f6..fb052e60c62 100644 --- a/code/lib/cli/src/autoblock/block-dependencies-versions.ts +++ b/code/lib/cli/src/autoblock/block-dependencies-versions.ts @@ -43,9 +43,6 @@ export const blocker = createBlocker({ return acc; }, false); }, - message(options, data) { - return `Found ${data.packageName} version: ${data.installedVersion}, please upgrade to ${data.minimumVersion} or higher.`; - }, log(options, data) { switch (data.packageName) { case 'react-scripts': diff --git a/code/lib/cli/src/autoblock/block-node-version.ts b/code/lib/cli/src/autoblock/block-node-version.ts index 220b29823e4..8d67cdd5101 100644 --- a/code/lib/cli/src/autoblock/block-node-version.ts +++ b/code/lib/cli/src/autoblock/block-node-version.ts @@ -11,9 +11,6 @@ export const blocker = createBlocker({ } return false; }, - message(options, data) { - return `Please use Node.js v18 or higher.`; - }, log(options, data) { return dedent` We've detected you're using Node.js v${data.nodeVersion}. diff --git a/code/lib/cli/src/autoblock/block-stories-mdx.ts b/code/lib/cli/src/autoblock/block-stories-mdx.ts index b868d913ecd..3c1fadeda35 100644 --- a/code/lib/cli/src/autoblock/block-stories-mdx.ts +++ b/code/lib/cli/src/autoblock/block-stories-mdx.ts @@ -11,12 +11,7 @@ export const blocker = createBlocker({ } return { files }; }, - message(options, data) { - return `Found ${data.files.length} stories.mdx ${ - data.files.length === 1 ? 'file' : 'files' - }, these must be migrated.`; - }, - log() { + log(options, data) { return dedent` Support for *.stories.mdx files has been removed. Please see the migration guide for more information: @@ -26,6 +21,10 @@ export const blocker = createBlocker({ Check the migration guide for more information: https://mdxjs.com/blog/v3/ + Found ${data.files.length} stories.mdx ${ + data.files.length === 1 ? 'file' : 'files' + }, these must be migrated. + Manually run the migration script to convert your stories.mdx files to CSF format documented here: https://storybook.js.org/docs/migration-guide#storiesmdx-to-mdxcsf `; diff --git a/code/lib/cli/src/autoblock/block-storystorev6.ts b/code/lib/cli/src/autoblock/block-storystorev6.ts index 40a9f8822ac..cd9eaffb6a7 100644 --- a/code/lib/cli/src/autoblock/block-storystorev6.ts +++ b/code/lib/cli/src/autoblock/block-storystorev6.ts @@ -1,4 +1,3 @@ -import { relative } from 'path'; import { createBlocker } from './types'; import { dedent } from 'ts-dedent'; import type { StorybookConfigRaw } from '@storybook/types'; @@ -15,10 +14,6 @@ export const blocker = createBlocker({ } return false; }, - message(options, data) { - const mainConfigPath = relative(process.cwd(), options.mainConfigPath); - return `StoryStoreV7 feature must be removed from ${mainConfigPath}`; - }, log() { return dedent` StoryStoreV7 feature must be removed from your Storybook configuration. diff --git a/code/lib/cli/src/autoblock/index.ts b/code/lib/cli/src/autoblock/index.ts index ca8116d890c..a6c45a2318b 100644 --- a/code/lib/cli/src/autoblock/index.ts +++ b/code/lib/cli/src/autoblock/index.ts @@ -2,7 +2,6 @@ import type { AutoblockOptions, Blocker } from './types'; import { logger } from '@storybook/node-logger'; import chalk from 'chalk'; import boxen from 'boxen'; -import { writeFile } from 'node:fs/promises'; const excludesFalse = (x: T | false): x is T => x !== false; @@ -34,7 +33,6 @@ export const autoblock = async ( return { id: blocker.id, value: true, - message: blocker.message(options, result), log: blocker.log(options, result), }; } else { @@ -46,12 +44,9 @@ export const autoblock = async ( const faults = out.filter(excludesFalse); if (faults.length > 0) { - const LOG_FILE_NAME = 'migration-storybook.log'; - const messages = { welcome: `Blocking your upgrade because of the following issues:`, reminder: chalk.yellow('Fix the above issues and try running the upgrade command again.'), - logfile: chalk.yellow(`You can find more details in ./${LOG_FILE_NAME}.`), }; const borderColor = '#FC521F'; @@ -59,22 +54,13 @@ export const autoblock = async ( logger.plain( boxen( [messages.welcome] - .concat(faults.map((i) => i.message)) + .concat(faults.map((i) => i.log)) .concat([messages.reminder]) - .concat([messages.logfile]) .join('\n\n'), { borderStyle: 'round', padding: 1, borderColor } ) ); - await writeFile( - LOG_FILE_NAME, - faults.map((i) => '(' + i.id + '):\n' + i.log).join('\n\n----\n\n'), - { - encoding: 'utf-8', - } - ); - return faults[0].id; } diff --git a/code/lib/cli/src/autoblock/types.ts b/code/lib/cli/src/autoblock/types.ts index 62be9625c76..39e6c728921 100644 --- a/code/lib/cli/src/autoblock/types.ts +++ b/code/lib/cli/src/autoblock/types.ts @@ -21,13 +21,6 @@ export interface Blocker { * @returns A truthy value to activate the block, return false to proceed. */ check: (options: AutoblockOptions) => Promise; - /** - * Format a message to be printed to the log-file. - * @param context - * @param data returned from the check method. - * @returns The string to print to the terminal. - */ - message: (options: AutoblockOptions, data: T) => string; /** * Format a message to be printed to the log-file. * @param context From 82d231b8a255b2ad5fbfe4565d4e8c6e5d7e060d Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Mon, 19 Feb 2024 16:13:19 +0100 Subject: [PATCH 2/4] apply changes to tests --- code/lib/cli/src/autoblock/index.test.ts | 36 +++++++++--------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/code/lib/cli/src/autoblock/index.test.ts b/code/lib/cli/src/autoblock/index.test.ts index ce5fa317041..265789ec7a0 100644 --- a/code/lib/cli/src/autoblock/index.test.ts +++ b/code/lib/cli/src/autoblock/index.test.ts @@ -2,7 +2,6 @@ import { expect, test, vi } from 'vitest'; import { autoblock } from './index'; import { JsPackageManagerFactory } from '@storybook/core-common'; import { createBlocker } from './types'; -import { writeFile as writeFileRaw } from 'node:fs/promises'; import { logger } from '@storybook/node-logger'; vi.mock('node:fs/promises', () => ({ @@ -19,26 +18,21 @@ vi.mock('@storybook/node-logger', () => ({ }, })); -const writeFile = vi.mocked(writeFileRaw); - const blockers = { alwaysPass: createBlocker({ id: 'alwaysPass', check: async () => false, - message: () => 'Always pass', log: () => 'Always pass', }), alwaysFail: createBlocker({ id: 'alwaysFail', check: async () => ({ bad: true }), - message: () => 'Always fail', - log: () => '...', + log: () => 'Always fail', }), alwaysFail2: createBlocker({ id: 'alwaysFail2', check: async () => ({ disaster: true }), - message: () => 'Always fail 2', - log: () => '...', + log: () => 'Always fail 2', }), } as const; @@ -75,17 +69,15 @@ test('1 fail', async () => { Promise.resolve({ blocker: blockers.alwaysPass }), Promise.resolve({ blocker: blockers.alwaysFail }), ]); - expect(writeFile).toHaveBeenCalledWith( - expect.any(String), - expect.stringContaining('alwaysFail'), - expect.any(Object) - ); + expect(result).toBe('alwaysFail'); expect(logger.plain).toHaveBeenCalledWith(expect.stringContaining('Oh no..')); + expect(logger.plain.mock.calls[1][0]).toMatchInlineSnapshot(` + "Blocking your upgrade because of the following issues: - expect(writeFile.mock.calls[0][1]).toMatchInlineSnapshot(` - "(alwaysFail): - ..." + Always fail + + Fix the above issues and try running the upgrade command again." `); }); @@ -95,14 +87,14 @@ test('multiple fails', async () => { Promise.resolve({ blocker: blockers.alwaysFail }), Promise.resolve({ blocker: blockers.alwaysFail2 }), ]); - expect(writeFile.mock.calls[0][1]).toMatchInlineSnapshot(` - "(alwaysFail): - ... + expect(logger.plain.mock.calls[1][0]).toMatchInlineSnapshot(` + "Blocking your upgrade because of the following issues: + + Always fail - ---- + Always fail 2 - (alwaysFail2): - ..." + Fix the above issues and try running the upgrade command again." `); expect(result).toBe('alwaysFail'); From 933be7fa7fdf4673cff65376cb417d2c981c4684 Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Mon, 19 Feb 2024 16:38:43 +0100 Subject: [PATCH 3/4] strip ansi to make it render the same on CI --- code/lib/cli/package.json | 1 + code/lib/cli/src/autoblock/index.test.ts | 13 ++++++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/code/lib/cli/package.json b/code/lib/cli/package.json index aee4d038a5e..35b9f511aa3 100644 --- a/code/lib/cli/package.json +++ b/code/lib/cli/package.json @@ -98,6 +98,7 @@ "@types/util-deprecate": "^1.0.0", "boxen": "^7.1.1", "slash": "^5.0.0", + "strip-ansi": "^7.1.0", "strip-json-comments": "^3.1.1", "typescript": "^5.3.2" }, diff --git a/code/lib/cli/src/autoblock/index.test.ts b/code/lib/cli/src/autoblock/index.test.ts index 265789ec7a0..18be67a4e6d 100644 --- a/code/lib/cli/src/autoblock/index.test.ts +++ b/code/lib/cli/src/autoblock/index.test.ts @@ -2,7 +2,8 @@ import { expect, test, vi } from 'vitest'; import { autoblock } from './index'; import { JsPackageManagerFactory } from '@storybook/core-common'; import { createBlocker } from './types'; -import { logger } from '@storybook/node-logger'; +import { logger as loggerRaw } from '@storybook/node-logger'; +import stripAnsi from 'strip-ansi'; vi.mock('node:fs/promises', () => ({ writeFile: vi.fn(), @@ -18,6 +19,8 @@ vi.mock('@storybook/node-logger', () => ({ }, })); +const logger = vi.mocked(loggerRaw); + const blockers = { alwaysPass: createBlocker({ id: 'alwaysPass', @@ -72,12 +75,12 @@ test('1 fail', async () => { expect(result).toBe('alwaysFail'); expect(logger.plain).toHaveBeenCalledWith(expect.stringContaining('Oh no..')); - expect(logger.plain.mock.calls[1][0]).toMatchInlineSnapshot(` + expect(stripAnsi(logger.plain.mock.calls[1][0])).toMatchInlineSnapshot(` "Blocking your upgrade because of the following issues: Always fail - Fix the above issues and try running the upgrade command again." + Fix the above issues and try running the upgrade command again." `); }); @@ -87,14 +90,14 @@ test('multiple fails', async () => { Promise.resolve({ blocker: blockers.alwaysFail }), Promise.resolve({ blocker: blockers.alwaysFail2 }), ]); - expect(logger.plain.mock.calls[1][0]).toMatchInlineSnapshot(` + expect(stripAnsi(logger.plain.mock.calls[1][0])).toMatchInlineSnapshot(` "Blocking your upgrade because of the following issues: Always fail Always fail 2 - Fix the above issues and try running the upgrade command again." + Fix the above issues and try running the upgrade command again." `); expect(result).toBe('alwaysFail'); From 125005381690e194f5e10ed158b9cf26fe8ac1ce Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Mon, 19 Feb 2024 16:39:04 +0100 Subject: [PATCH 4/4] fix lockfile --- code/yarn.lock | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/code/yarn.lock b/code/yarn.lock index 86f09cd405d..74fa8877ed0 100644 --- a/code/yarn.lock +++ b/code/yarn.lock @@ -5390,6 +5390,7 @@ __metadata: read-pkg-up: "npm:^7.0.1" semver: "npm:^7.3.7" slash: "npm:^5.0.0" + strip-ansi: "npm:^7.1.0" strip-json-comments: "npm:^3.1.1" tempy: "npm:^1.0.1" tiny-invariant: "npm:^1.3.1" @@ -26793,7 +26794,7 @@ __metadata: languageName: node linkType: hard -"strip-ansi@npm:^7.0.0, strip-ansi@npm:^7.0.1": +"strip-ansi@npm:^7.0.0, strip-ansi@npm:^7.0.1, strip-ansi@npm:^7.1.0": version: 7.1.0 resolution: "strip-ansi@npm:7.1.0" dependencies: