From a476108e33ec17c414a46f432c0c1d35b89efe08 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Mon, 2 Sep 2024 13:17:54 +0200 Subject: [PATCH 1/2] CLI: Handle Yarn PnP wrapper scenario when adding an addon --- code/lib/cli-storybook/src/add.ts | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/code/lib/cli-storybook/src/add.ts b/code/lib/cli-storybook/src/add.ts index df3ac84f140f..acc259c83b55 100644 --- a/code/lib/cli-storybook/src/add.ts +++ b/code/lib/cli-storybook/src/add.ts @@ -13,6 +13,11 @@ import { readConfig, writeConfig } from 'storybook/internal/csf-tools'; import SemVer from 'semver'; import { dedent } from 'ts-dedent'; +import { + getRequireWrapperName, + isRequireWrapperNecessary, + wrapValueWithRequireWrapper, +} from './automigrate/fixes/wrap-require-utils'; import { postinstallAddon } from './postinstallAddon'; export interface PostinstallOptions { @@ -136,7 +141,17 @@ export async function add( await packageManager.addDependencies({ installAsDevDependencies: true }, [addonWithVersion]); logger.log(`Adding '${addon}' to main.js addons field.`); - main.appendValueToArray(['addons'], addonName); + + const mainConfigAddons = main.getFieldNode(['addons']); + + if (mainConfigAddons && getRequireWrapperName(main) !== null) { + const addonNode = main.valueToNode(addonName); + main.appendNodeToArray(['addons'], addonNode as any); + wrapValueWithRequireWrapper(main, addonNode as any); + } else { + main.appendValueToArray(['addons'], addonName); + } + await writeConfig(main); if (!skipPostinstall && isCoreAddon(addonName)) { From 57af7c05dd582cc985c4d97103deceb5688b6016 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Mon, 2 Sep 2024 14:34:04 +0200 Subject: [PATCH 2/2] add tests --- code/lib/cli-storybook/src/add.test.ts | 43 +++++++++++++++++++++++++- code/lib/cli-storybook/src/add.ts | 2 -- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/code/lib/cli-storybook/src/add.test.ts b/code/lib/cli-storybook/src/add.test.ts index 137e60a72008..5fb009f11df3 100644 --- a/code/lib/cli-storybook/src/add.test.ts +++ b/code/lib/cli-storybook/src/add.test.ts @@ -1,10 +1,13 @@ -import { describe, expect, test, vi } from 'vitest'; +import { beforeEach, describe, expect, test, vi } from 'vitest'; import { add, getVersionSpecifier } from './add'; const MockedConfig = vi.hoisted(() => { return { appendValueToArray: vi.fn(), + getFieldNode: vi.fn(), + valueToNode: vi.fn(), + appendNodeToArray: vi.fn(), }; }); const MockedPackageManager = vi.hoisted(() => { @@ -20,6 +23,12 @@ const MockedPostInstall = vi.hoisted(() => { postinstallAddon: vi.fn(), }; }); +const MockWrapRequireUtils = vi.hoisted(() => { + return { + getRequireWrapperName: vi.fn(), + wrapValueWithRequireWrapper: vi.fn(), + }; +}); const MockedConsole = { log: vi.fn(), warn: vi.fn(), @@ -35,6 +44,9 @@ vi.mock('storybook/internal/csf-tools', () => { vi.mock('./postinstallAddon', () => { return MockedPostInstall; }); +vi.mock('./automigrate/fixes/wrap-require-utils', () => { + return MockWrapRequireUtils; +}); vi.mock('storybook/internal/common', () => { return { getStorybookInfo: vi.fn(() => ({ mainConfig: {}, configDir: '' })), @@ -103,6 +115,35 @@ describe('add', () => { }); describe('add (extra)', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + test('should not add a "wrap require" to the addon when not needed', async () => { + MockedConfig.getFieldNode.mockReturnValue({}); + MockWrapRequireUtils.getRequireWrapperName.mockReturnValue(null); + await add( + '@storybook/addon-docs', + { packageManager: 'npm', skipPostinstall: true }, + MockedConsole + ); + + expect(MockWrapRequireUtils.wrapValueWithRequireWrapper).not.toHaveBeenCalled(); + expect(MockedConfig.appendValueToArray).toHaveBeenCalled(); + expect(MockedConfig.appendNodeToArray).not.toHaveBeenCalled(); + }); + test('should add a "wrap require" to the addon when applicable', async () => { + MockedConfig.getFieldNode.mockReturnValue({}); + MockWrapRequireUtils.getRequireWrapperName.mockReturnValue('require'); + await add( + '@storybook/addon-docs', + { packageManager: 'npm', skipPostinstall: true }, + MockedConsole + ); + + expect(MockWrapRequireUtils.wrapValueWithRequireWrapper).toHaveBeenCalled(); + expect(MockedConfig.appendValueToArray).not.toHaveBeenCalled(); + expect(MockedConfig.appendNodeToArray).toHaveBeenCalled(); + }); test('not warning when installing the correct version of storybook', async () => { await add( '@storybook/addon-docs', diff --git a/code/lib/cli-storybook/src/add.ts b/code/lib/cli-storybook/src/add.ts index acc259c83b55..d0e64efdf443 100644 --- a/code/lib/cli-storybook/src/add.ts +++ b/code/lib/cli-storybook/src/add.ts @@ -15,7 +15,6 @@ import { dedent } from 'ts-dedent'; import { getRequireWrapperName, - isRequireWrapperNecessary, wrapValueWithRequireWrapper, } from './automigrate/fixes/wrap-require-utils'; import { postinstallAddon } from './postinstallAddon'; @@ -143,7 +142,6 @@ export async function add( logger.log(`Adding '${addon}' to main.js addons field.`); const mainConfigAddons = main.getFieldNode(['addons']); - if (mainConfigAddons && getRequireWrapperName(main) !== null) { const addonNode = main.valueToNode(addonName); main.appendNodeToArray(['addons'], addonNode as any);