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: Add "missing-storybook-dependencies" automigration #28579

Merged
merged 12 commits into from
Jul 15, 2024
4 changes: 4 additions & 0 deletions code/core/src/common/js-package-manager/JsPackageManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,10 @@ export abstract class JsPackageManager {
stdio?: 'inherit' | 'pipe'
): string;
public abstract findInstallations(pattern?: string[]): Promise<InstallationMetadata | undefined>;
public abstract findInstallations(
pattern?: string[],
options?: { depth: number }
): Promise<InstallationMetadata | undefined>;
public abstract parseErrorFromLogs(logs?: string): string;

public executeCommandSync({
Expand Down
10 changes: 5 additions & 5 deletions code/core/src/common/js-package-manager/NPMProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,28 +132,28 @@ export class NPMProxy extends JsPackageManager {
});
}

public async findInstallations(pattern: string[]) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea of adding a depth option is because we want to try and get information over dependencies that are coming directly from package.json, but NOT as dependencies of dependencies. This needs further testing on all package managers

const exec = async ({ depth }: { depth: number }) => {
public async findInstallations(pattern: string[], { depth = 99 }: { depth?: number } = {}) {
const exec = async ({ packageDepth }: { packageDepth: number }) => {
const pipeToNull = platform() === 'win32' ? '2>NUL' : '2>/dev/null';
return this.executeCommand({
command: 'npm',
args: ['ls', '--json', `--depth=${depth}`, pipeToNull],
args: ['ls', '--json', `--depth=${packageDepth}`, pipeToNull],
env: {
FORCE_COLOR: 'false',
},
});
};

try {
const commandResult = await exec({ depth: 99 });
const commandResult = await exec({ packageDepth: depth });
const parsedOutput = JSON.parse(commandResult);

return this.mapDependencies(parsedOutput, pattern);
} catch (e) {
// when --depth is higher than 0, npm can return a non-zero exit code
// in case the user's project has peer dependency issues. So we try again with no depth
try {
const commandResult = await exec({ depth: 0 });
const commandResult = await exec({ packageDepth: 0 });
const parsedOutput = JSON.parse(commandResult);

return this.mapDependencies(parsedOutput, pattern);
Expand Down
18 changes: 9 additions & 9 deletions code/core/src/common/js-package-manager/PNPMProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,16 +98,16 @@ export class PNPMProxy extends JsPackageManager {
});
}

public async findInstallations(pattern: string[]) {
const commandResult = await this.executeCommand({
command: 'pnpm',
args: ['list', pattern.map((p) => `"${p}"`).join(' '), '--json', '--depth=99'],
env: {
FORCE_COLOR: 'false',
},
});

public async findInstallations(pattern: string[], { depth = 99 }: { depth?: number } = {}) {
try {
const commandResult = await this.executeCommand({
command: 'pnpm',
args: ['list', pattern.map((p) => `"${p}"`).join(' '), '--json', `--depth=${depth}`],
env: {
FORCE_COLOR: 'false',
},
});

const parsedOutput = JSON.parse(commandResult);
return this.mapDependencies(parsedOutput, pattern);
} catch (e) {
Expand Down
22 changes: 14 additions & 8 deletions code/core/src/common/js-package-manager/Yarn1Proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,16 +83,22 @@ export class Yarn1Proxy extends JsPackageManager {
return JSON.parse(readFileSync(packageJsonPath, 'utf-8')) as Record<string, any>;
}

public async findInstallations(pattern: string[]) {
const commandResult = await this.executeCommand({
command: 'yarn',
args: ['list', '--pattern', pattern.map((p) => `"${p}"`).join(' '), '--recursive', '--json'],
env: {
FORCE_COLOR: 'false',
},
});
public async findInstallations(pattern: string[], { depth = 99 }: { depth?: number } = {}) {
const yarnArgs = ['list', '--pattern', pattern.map((p) => `"${p}"`).join(' '), '--json'];

if (depth !== 0) {
yarnArgs.push('--recursive');
}

try {
const commandResult = await this.executeCommand({
command: 'yarn',
args: yarnArgs.concat(pattern),
env: {
FORCE_COLOR: 'false',
},
});

const parsedOutput = JSON.parse(commandResult);
return this.mapDependencies(parsedOutput, pattern);
} catch (e) {
Expand Down
22 changes: 14 additions & 8 deletions code/core/src/common/js-package-manager/Yarn2Proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,16 +120,22 @@ export class Yarn2Proxy extends JsPackageManager {
return this.executeCommand({ command: 'yarn', args: [command, ...args], cwd });
}

public async findInstallations(pattern: string[]) {
const commandResult = await this.executeCommand({
command: 'yarn',
args: ['info', '--name-only', '--recursive', ...pattern],
env: {
FORCE_COLOR: 'false',
},
});
public async findInstallations(pattern: string[], { depth = 99 }: { depth?: number } = {}) {
const yarnArgs = ['info', '--name-only'];

if (depth !== 0) {
yarnArgs.push('--recursive');
}

try {
const commandResult = await this.executeCommand({
command: 'yarn',
args: yarnArgs.concat(pattern),
env: {
FORCE_COLOR: 'false',
},
});

return this.mapDependencies(commandResult, pattern);
} catch (e) {
return undefined;
Expand Down
2 changes: 2 additions & 0 deletions code/lib/cli/src/automigrate/fixes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,12 @@ import { vta } from './vta';
import { upgradeStorybookRelatedDependencies } from './upgrade-storybook-related-dependencies';
import { autodocsTags } from './autodocs-tags';
import { initialGlobals } from './initial-globals';
import { missingStorybookDependencies } from './missing-storybook-dependencies';

export * from '../types';

export const allFixes: Fix[] = [
missingStorybookDependencies,
addonsAPI,
newFrameworks,
cra5,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import { describe, expect, vi, it, beforeEach } from 'vitest';
import type { JsPackageManager } from '@storybook/core/common';
import stripAnsi from 'strip-ansi';

import { missingStorybookDependencies } from './missing-storybook-dependencies';

vi.mock('globby', () => ({
__esModule: true,
globby: vi.fn().mockResolvedValue(['.storybook/manager.ts', 'path/to/file.stories.tsx']),
}));

vi.mock('node:fs/promises', () => ({
__esModule: true,
readFile: vi.fn().mockResolvedValue(`
// these are NOT installed, will be reported
import { someFunction } from '@storybook/preview-api';
import { anotherFunction } from '@storybook/manager-api';
import { SomeError } from '@storybook/core-events/server-errors';
// this IS installed, will not be reported
import { yetAnotherFunction } from '@storybook/theming';
`),
}));

vi.mock('../../helpers', () => ({
getStorybookVersionSpecifier: vi.fn().mockReturnValue('^8.1.10'),
}));

const check = async ({
packageManager,
storybookVersion = '8.1.10',
}: {
packageManager: JsPackageManager;
storybookVersion?: string;
}) => {
return missingStorybookDependencies.check({
packageManager,
mainConfig: {} as any,
storybookVersion,
});
};

describe('missingStorybookDependencies', () => {
const mockPackageManager = {
findInstallations: vi.fn().mockResolvedValue({
dependencies: {
'@storybook/react': '8.1.0',
'@storybook/theming': '8.1.0',
},
}),
retrievePackageJson: vi.fn().mockResolvedValue({
dependencies: {
'@storybook/core': '8.1.0',
},
}),
addDependencies: vi.fn().mockResolvedValue(undefined),
} as Partial<JsPackageManager>;

describe('check function', () => {
it('should identify missing dependencies', async () => {
const result = await check({
packageManager: mockPackageManager as JsPackageManager,
});

expect(Object.keys(result!.packageUsage)).not.includes('@storybook/theming');
yannbf marked this conversation as resolved.
Show resolved Hide resolved
expect(result).toEqual({
packageUsage: {
'@storybook/preview-api': ['.storybook/manager.ts', 'path/to/file.stories.tsx'],
'@storybook/manager-api': ['.storybook/manager.ts', 'path/to/file.stories.tsx'],
'@storybook/core-events': ['.storybook/manager.ts', 'path/to/file.stories.tsx'],
},
});
});
});

describe('prompt function', () => {
it('should provide a proper message with the missing dependencies', () => {
const packageUsage = {
'@storybook/preview-api': ['.storybook/manager.ts'],
'@storybook/manager-api': ['path/to/file.stories.tsx'],
};

const message = missingStorybookDependencies.prompt({ packageUsage });

expect(stripAnsi(message)).toMatchInlineSnapshot(`
"Found the following Storybook packages used in your project, but they are missing from your project dependencies:
- @storybook/manager-api: (1 file)
- @storybook/preview-api: (1 file)

Referencing missing packages can cause your project to crash. We can automatically add them to your dependencies.

More info: https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#failed-to-resolve-import-storybookx-error"
`);
});
});

describe('run function', () => {
it('should add missing dependencies', async () => {
const dryRun = false;
const packageUsage = {
'@storybook/preview-api': ['.storybook/manager.ts'],
'@storybook/manager-api': ['path/to/file.stories.tsx'],
};

await missingStorybookDependencies.run!({
result: { packageUsage },
dryRun,
packageManager: mockPackageManager as JsPackageManager,
mainConfigPath: 'path/to/main-config.js',
});

expect(mockPackageManager.addDependencies).toHaveBeenNthCalledWith(
1,
{ installAsDevDependencies: true },
['@storybook/preview-api@8.1.10', '@storybook/manager-api@8.1.10']
);
expect(mockPackageManager.addDependencies).toHaveBeenNthCalledWith(
2,
{ installAsDevDependencies: true, skipInstall: true, packageJson: expect.anything() },
['@storybook/preview-api@^8.1.10', '@storybook/manager-api@^8.1.10']
);
});
});
});
Loading