Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLI: Remove the logging to file feature from autoblockers #26100

Merged
merged 7 commits into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -26878,7 +26879,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
Loading