Skip to content

Commit

Permalink
Merge pull request #28579 from storybookjs/yann/missing-storybook-dep…
Browse files Browse the repository at this point in the history
…s-automigration

CLI: Add "missing-storybook-dependencies" automigration
(cherry picked from commit 8dc43db)
  • Loading branch information
shilman authored and storybook-bot committed Jul 15, 2024
1 parent c91a4ba commit 48286d0
Show file tree
Hide file tree
Showing 9 changed files with 329 additions and 32 deletions.
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[]) {
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 @@ -121,16 +121,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');
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

0 comments on commit 48286d0

Please sign in to comment.