Skip to content

Commit

Permalink
Merge pull request #26100 from storybookjs/norbert/remove-logging-fea…
Browse files Browse the repository at this point in the history
…ture-from-autoblockers

CLI: Remove the logging to file feature from autoblockers
  • Loading branch information
ndelangen authored Feb 20, 2024
2 parents 2cc319a + c827d17 commit b7451ae
Show file tree
Hide file tree
Showing 9 changed files with 26 additions and 62 deletions.
1 change: 1 addition & 0 deletions code/lib/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,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"
},
Expand Down
3 changes: 0 additions & 3 deletions code/lib/cli/src/autoblock/block-dependencies-versions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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':
Expand Down
3 changes: 0 additions & 3 deletions code/lib/cli/src/autoblock/block-node-version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
Expand Down
11 changes: 5 additions & 6 deletions code/lib/cli/src/autoblock/block-stories-mdx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
`;
Expand Down
5 changes: 0 additions & 5 deletions code/lib/cli/src/autoblock/block-storystorev6.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { relative } from 'path';
import { createBlocker } from './types';
import { dedent } from 'ts-dedent';
import type { StorybookConfigRaw } from '@storybook/types';
Expand All @@ -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.
Expand Down
39 changes: 17 additions & 22 deletions code/lib/cli/src/autoblock/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ 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';
import { logger as loggerRaw } from '@storybook/node-logger';
import stripAnsi from 'strip-ansi';

vi.mock('node:fs/promises', () => ({
writeFile: vi.fn(),
Expand All @@ -19,26 +19,23 @@ vi.mock('@storybook/node-logger', () => ({
},
}));

const writeFile = vi.mocked(writeFileRaw);
const logger = vi.mocked(loggerRaw);

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;

Expand Down Expand Up @@ -75,17 +72,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(stripAnsi(logger.plain.mock.calls[1][0])).toMatchInlineSnapshot(`
"Blocking your upgrade because of the following issues:
Always fail
expect(writeFile.mock.calls[0][1]).toMatchInlineSnapshot(`
"(alwaysFail):
..."
Fix the above issues and try running the upgrade command again."
`);
});

Expand All @@ -95,14 +90,14 @@ test('multiple fails', async () => {
Promise.resolve({ blocker: blockers.alwaysFail }),
Promise.resolve({ blocker: blockers.alwaysFail2 }),
]);
expect(writeFile.mock.calls[0][1]).toMatchInlineSnapshot(`
"(alwaysFail):
...
expect(stripAnsi(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');
Expand Down
16 changes: 1 addition & 15 deletions code/lib/cli/src/autoblock/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = <T>(x: T | false): x is T => x !== false;

Expand Down Expand Up @@ -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 {
Expand All @@ -46,35 +44,23 @@ 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';

logger.plain('Oh no..');
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;
}

Expand Down
7 changes: 0 additions & 7 deletions code/lib/cli/src/autoblock/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,6 @@ export interface Blocker<T> {
* @returns A truthy value to activate the block, return false to proceed.
*/
check: (options: AutoblockOptions) => Promise<T | false>;
/**
* 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
Expand Down
3 changes: 2 additions & 1 deletion code/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5474,6 +5474,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"
Expand Down Expand Up @@ -26879,7 +26880,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:
Expand Down

0 comments on commit b7451ae

Please sign in to comment.