From 6f6f8d7d8d526e31cba62ab220c48e7c28e3d7fd Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Sat, 24 Jun 2023 00:57:56 +0800 Subject: [PATCH] Merge pull request #23199 from storybookjs/fix/parse-pnp-paths-in-metadata CLI: Parse pnp paths in storybook metadata (cherry picked from commit 4f7cd07e9b4e2da6b347c36de033df8463108167) --- code/lib/telemetry/src/get-framework-info.ts | 34 +- code/lib/telemetry/src/sanitize.test.ts | 8 +- .../telemetry/src/storybook-metadata.test.ts | 445 +++++++++++------- 3 files changed, 313 insertions(+), 174 deletions(-) diff --git a/code/lib/telemetry/src/get-framework-info.ts b/code/lib/telemetry/src/get-framework-info.ts index 4f465c7f0b2f..c5e706166471 100644 --- a/code/lib/telemetry/src/get-framework-info.ts +++ b/code/lib/telemetry/src/get-framework-info.ts @@ -1,4 +1,7 @@ import type { PackageJson, StorybookConfig } from '@storybook/types'; +import path from 'path'; +import { frameworkPackages } from '@storybook/core-common'; +import { cleanPaths } from './sanitize'; import { getActualPackageJson } from './package-json'; const knownRenderers = [ @@ -30,20 +33,39 @@ function findMatchingPackage(packageJson: PackageJson, suffixes: string[]) { return suffixes.map((suffix) => `@storybook/${suffix}`).find((pkg) => allDependencies[pkg]); } -export async function getFrameworkInfo(mainConfig: StorybookConfig) { - const { framework: frameworkInput } = mainConfig; +export const getFrameworkPackageName = (mainConfig?: StorybookConfig) => { + const packageNameOrPath = + typeof mainConfig?.framework === 'string' ? mainConfig.framework : mainConfig?.framework?.name; + + if (!packageNameOrPath) { + return null; + } + + const normalizedPath = path.normalize(packageNameOrPath).replace(new RegExp(/\\/, 'g'), '/'); - if (!frameworkInput) return {}; + const knownFramework = Object.keys(frameworkPackages).find((pkg) => normalizedPath.endsWith(pkg)); + + return knownFramework || cleanPaths(packageNameOrPath).replace(/.*node_modules[\\/]/, ''); +}; + +export async function getFrameworkInfo(mainConfig: StorybookConfig) { + if (!mainConfig.framework) return {}; - const framework = typeof frameworkInput === 'string' ? { name: frameworkInput } : frameworkInput; + const frameworkName = getFrameworkPackageName(mainConfig); + if (!frameworkName) return {}; + const frameworkOptions = + typeof mainConfig.framework === 'object' ? mainConfig.framework.options : {}; - const frameworkPackageJson = await getActualPackageJson(framework.name); + const frameworkPackageJson = await getActualPackageJson(frameworkName); const builder = findMatchingPackage(frameworkPackageJson, knownBuilders); const renderer = findMatchingPackage(frameworkPackageJson, knownRenderers); return { - framework, + framework: { + name: frameworkName, + options: frameworkOptions, + }, builder, renderer, }; diff --git a/code/lib/telemetry/src/sanitize.test.ts b/code/lib/telemetry/src/sanitize.test.ts index 2f8f5b2267f7..f5b3a742d3e6 100644 --- a/code/lib/telemetry/src/sanitize.test.ts +++ b/code/lib/telemetry/src/sanitize.test.ts @@ -73,10 +73,10 @@ describe(`Errors Helpers`, () => { const mockCwd = jest.spyOn(process, `cwd`).mockImplementation(() => cwdMockPath); - const errorMessage = `This path ${fullPath} is a test ${fullPath}`; + const errorMessage = `This path should be sanitized ${fullPath}`; expect(cleanPaths(errorMessage, `/`)).toBe( - `This path $SNIP/${filePath} is a test $SNIP/${filePath}` + `This path should be sanitized $SNIP/${filePath}` ); mockCwd.mockRestore(); } @@ -90,10 +90,10 @@ describe(`Errors Helpers`, () => { const mockCwd = jest.spyOn(process, `cwd`).mockImplementation(() => cwdMockPath); - const errorMessage = `This path ${fullPath} is a test ${fullPath}`; + const errorMessage = `This path should be sanitized ${fullPath}`; expect(cleanPaths(errorMessage, `\\`)).toBe( - `This path $SNIP\\${filePath} is a test $SNIP\\${filePath}` + `This path should be sanitized $SNIP\\${filePath}` ); mockCwd.mockRestore(); } diff --git a/code/lib/telemetry/src/storybook-metadata.test.ts b/code/lib/telemetry/src/storybook-metadata.test.ts index 609cee560c14..ab64dc26e9a1 100644 --- a/code/lib/telemetry/src/storybook-metadata.test.ts +++ b/code/lib/telemetry/src/storybook-metadata.test.ts @@ -47,209 +47,326 @@ jest.mock('detect-package-manager', () => ({ getNpmVersion: () => '3.1.1', })); -describe('sanitizeAddonName', () => { - const originalSep = path.sep; +const originalSep = path.sep; + +describe('storybook-metadata', () => { + let cwdSpy: jest.SpyInstance; beforeEach(() => { // @ts-expect-error the property is read only but we can change it for testing purposes path.sep = originalSep; }); - test('special addon names', () => { - const addonNames = [ - '@storybook/preset-create-react-app', - 'storybook-addon-deprecated/register', - 'storybook-addon-ends-with-js/register.js', - '@storybook/addon-knobs/preset', - '@storybook/addon-ends-with-js/preset.js', - '@storybook/addon-postcss/dist/index.js', - '../local-addon/register.js', - '../../', - ].map(sanitizeAddonName); - - expect(addonNames).toEqual([ - '@storybook/preset-create-react-app', - 'storybook-addon-deprecated', - 'storybook-addon-ends-with-js', - '@storybook/addon-knobs', - '@storybook/addon-ends-with-js', - '@storybook/addon-postcss', - '../local-addon', - '../../', - ]); + afterEach(() => { + cwdSpy?.mockRestore(); + // @ts-expect-error the property is read only but we can change it for testing purposes + path.sep = originalSep; }); - test('Windows paths', () => { - // @ts-expect-error the property is read only but we can change it for testing purposes - path.sep = '\\'; - const cwdMockPath = `C:\\Users\\username\\storybook-app`; - jest.spyOn(process, `cwd`).mockImplementationOnce(() => cwdMockPath); + describe('sanitizeAddonName', () => { + test('special addon names', () => { + const addonNames = [ + '@storybook/preset-create-react-app', + 'storybook-addon-deprecated/register', + 'storybook-addon-ends-with-js/register.js', + '@storybook/addon-knobs/preset', + '@storybook/addon-ends-with-js/preset.js', + '@storybook/addon-postcss/dist/index.js', + '../local-addon/register.js', + '../../', + ].map(sanitizeAddonName); - expect(sanitizeAddonName(`${cwdMockPath}\\local-addon\\themes.js`)).toEqual( - '$SNIP\\local-addon\\themes' - ); - }); + expect(addonNames).toEqual([ + '@storybook/preset-create-react-app', + 'storybook-addon-deprecated', + 'storybook-addon-ends-with-js', + '@storybook/addon-knobs', + '@storybook/addon-ends-with-js', + '@storybook/addon-postcss', + '../local-addon', + '../../', + ]); + }); - test('Linux paths', () => { - // @ts-expect-error the property is read only but we can change it for testing purposes - path.sep = '/'; - const cwdMockPath = `/Users/username/storybook-app`; - jest.spyOn(process, `cwd`).mockImplementationOnce(() => cwdMockPath); + test('Windows paths', () => { + // @ts-expect-error the property is read only but we can change it for testing purposes + path.sep = '\\'; + const cwdMockPath = `C:\\Users\\username\\storybook-app`; + cwdSpy = jest.spyOn(process, `cwd`).mockReturnValueOnce(cwdMockPath); - expect(sanitizeAddonName(`${cwdMockPath}/local-addon/themes.js`)).toEqual( - '$SNIP/local-addon/themes' - ); + expect(sanitizeAddonName(`${cwdMockPath}\\local-addon\\themes.js`)).toEqual( + '$SNIP\\local-addon\\themes' + ); + }); + + test('Linux paths', () => { + // @ts-expect-error the property is read only but we can change it for testing purposes + path.sep = '/'; + const cwdMockPath = `/Users/username/storybook-app`; + cwdSpy = jest.spyOn(process, `cwd`).mockReturnValue(cwdMockPath); + + expect(sanitizeAddonName(`${cwdMockPath}/local-addon/themes.js`)).toEqual( + '$SNIP/local-addon/themes' + ); + }); }); -}); -describe('await computeStorybookMetadata', () => { - test('should return frameworkOptions from mainjs', async () => { - const reactResult = await computeStorybookMetadata({ - packageJson: packageJsonMock, - mainConfig: { - ...mainJsMock, - framework: { + describe('computeStorybookMetadata', () => { + describe('pnp paths', () => { + test('should parse pnp paths for known frameworks', async () => { + const unixResult = await computeStorybookMetadata({ + packageJson: packageJsonMock, + mainConfig: { + ...mainJsMock, + framework: { + name: '/Users/foo/storybook/.yarn/__virtual__/@storybook-react-vite-virtual-769c990b9/0/cache/@storybook-react-vite-npm-7.1.0-alpha.38-512b-a23.zip/node_modules/@storybook/react-vite', + options: { + fastRefresh: false, + }, + }, + }, + }); + + expect(unixResult.framework).toEqual({ name: '@storybook/react-vite', - options: { - fastRefresh: false, + options: { fastRefresh: false }, + }); + + const windowsResult = await computeStorybookMetadata({ + packageJson: packageJsonMock, + mainConfig: { + ...mainJsMock, + framework: { + name: 'C:\\Users\\foo\\storybook\\.yarn\\__virtual__\\@storybook-react-vite-virtual-769c990b9\\0\\cache\\@storybook-react-vite-npm-7.1.0-alpha.38-512b-a23.zip\\node_modules\\@storybook\\react-vite', + options: { + fastRefresh: false, + }, + }, }, - }, - }, - }); + }); + + expect(windowsResult.framework).toEqual({ + name: '@storybook/react-vite', + options: { fastRefresh: false }, + }); + }); - expect(reactResult.framework).toEqual({ - name: '@storybook/react-vite', - options: { fastRefresh: false }, + test('should parse pnp paths for unknown frameworks', async () => { + const unixResult = await computeStorybookMetadata({ + packageJson: packageJsonMock, + mainConfig: { + ...mainJsMock, + framework: { + name: '/Users/foo/my-project/.yarn/__virtual__/@storybook-react-vite-virtual-769c990b9/0/cache/@storybook-react-rust-npm-7.1.0-alpha.38-512b-a23.zip/node_modules/storybook-react-rust', + }, + }, + }); + + expect(unixResult.framework).toEqual({ + name: 'storybook-react-rust', + }); + + const windowsResult = await computeStorybookMetadata({ + packageJson: packageJsonMock, + mainConfig: { + ...mainJsMock, + framework: { + name: 'C:\\Users\\foo\\my-project\\.yarn\\__virtual__\\@storybook-react-vite-virtual-769c990b9\\0\\cache\\@storybook-react-rust-npm-7.1.0-alpha.38-512b-a23.zip\\node_modules\\storybook-react-rust', + }, + }, + }); + + expect(windowsResult.framework).toEqual({ + name: 'storybook-react-rust', + }); + }); + + test('should sanitize pnp paths for local frameworks', async () => { + // @ts-expect-error the property is read only but we can change it for testing purposes + path.sep = '/'; + cwdSpy = jest.spyOn(process, 'cwd').mockReturnValue('/Users/foo/my-projects'); + + const unixResult = await computeStorybookMetadata({ + packageJson: packageJsonMock, + mainConfig: { + ...mainJsMock, + framework: { + name: '/Users/foo/my-projects/.storybook/some-local-framework', + }, + }, + }); + + expect(unixResult.framework).toEqual({ + name: '$SNIP/.storybook/some-local-framework', + }); + + // @ts-expect-error the property is read only but we can change it for testing purposes + path.sep = '\\'; + cwdSpy = jest.spyOn(process, 'cwd').mockReturnValue('C:\\Users\\foo\\my-project'); + const windowsResult = await computeStorybookMetadata({ + packageJson: packageJsonMock, + mainConfig: { + ...mainJsMock, + framework: { + name: 'C:\\Users\\foo\\my-project\\.storybook\\some-local-framework', + }, + }, + }); + + expect(windowsResult.framework).toEqual({ + name: '$SNIP\\.storybook\\some-local-framework', + }); + }); }); - const angularResult = await computeStorybookMetadata({ - packageJson: packageJsonMock, - mainConfig: { - ...mainJsMock, - framework: { - name: '@storybook/angular', - options: { - enableIvy: true, - enableNgcc: true, + test('should return frameworkOptions from mainjs', async () => { + const reactResult = await computeStorybookMetadata({ + packageJson: packageJsonMock, + mainConfig: { + ...mainJsMock, + framework: { + name: '@storybook/react-vite', + options: { + fastRefresh: false, + }, }, }, - }, - }); + }); - expect(angularResult.framework).toEqual({ - name: '@storybook/angular', - options: { enableIvy: true, enableNgcc: true }, - }); - }); + expect(reactResult.framework).toEqual({ + name: '@storybook/react-vite', + options: { fastRefresh: false }, + }); - test('should separate storybook packages and addons', async () => { - const result = await computeStorybookMetadata({ - packageJson: { - ...packageJsonMock, - devDependencies: { - '@storybook/react': 'x.y.z', - '@storybook/addon-essentials': 'x.x.x', - '@storybook/addon-knobs': 'x.x.y', - 'storybook-addon-deprecated': 'x.x.z', + const angularResult = await computeStorybookMetadata({ + packageJson: packageJsonMock, + mainConfig: { + ...mainJsMock, + framework: { + name: '@storybook/angular', + options: { + enableIvy: true, + enableNgcc: true, + }, + }, }, - }, - mainConfig: { - ...mainJsMock, - addons: [ - '@storybook/addon-essentials', - 'storybook-addon-deprecated/register', - '@storybook/addon-knobs/preset', - ], - }, + }); + + expect(angularResult.framework).toEqual({ + name: '@storybook/angular', + options: { enableIvy: true, enableNgcc: true }, + }); }); - expect(result.addons).toMatchInlineSnapshot(` - Object { - "@storybook/addon-essentials": Object { - "options": undefined, - "version": "x.x.x", - }, - "@storybook/addon-knobs": Object { - "options": undefined, - "version": "x.x.x", - }, - "storybook-addon-deprecated": Object { - "options": undefined, - "version": "x.x.x", - }, - } - `); - expect(result.storybookPackages).toMatchInlineSnapshot(` - Object { - "@storybook/react": Object { - "version": "x.x.x", + test('should separate storybook packages and addons', async () => { + const result = await computeStorybookMetadata({ + packageJson: { + ...packageJsonMock, + devDependencies: { + '@storybook/react': 'x.y.z', + '@storybook/addon-essentials': 'x.x.x', + '@storybook/addon-knobs': 'x.x.y', + 'storybook-addon-deprecated': 'x.x.z', + }, + } as PackageJson, + mainConfig: { + ...mainJsMock, + addons: [ + '@storybook/addon-essentials', + 'storybook-addon-deprecated/register', + '@storybook/addon-knobs/preset', + ], }, - } - `); - }); + }); - test('should return user specified features', async () => { - const features = { - storyStoreV7: true, - }; - - const result = await computeStorybookMetadata({ - packageJson: packageJsonMock, - mainConfig: { - ...mainJsMock, - features, - }, + expect(result.addons).toMatchInlineSnapshot(` + Object { + "@storybook/addon-essentials": Object { + "options": undefined, + "version": "x.x.x", + }, + "@storybook/addon-knobs": Object { + "options": undefined, + "version": "x.x.x", + }, + "storybook-addon-deprecated": Object { + "options": undefined, + "version": "x.x.x", + }, + } + `); + expect(result.storybookPackages).toMatchInlineSnapshot(` + Object { + "@storybook/react": Object { + "version": "x.x.x", + }, + } + `); }); - expect(result.features).toEqual(features); - }); + test('should return user specified features', async () => { + const features = { + storyStoreV7: true, + }; - test('should infer builder and renderer from framework package.json', async () => { - expect( - await computeStorybookMetadata({ + const result = await computeStorybookMetadata({ packageJson: packageJsonMock, mainConfig: { ...mainJsMock, - framework: '@storybook/react-vite', + features, }, - }) - ).toMatchObject({ - framework: { name: '@storybook/react-vite' }, - renderer: '@storybook/react', - builder: '@storybook/builder-vite', + }); + + expect(result.features).toEqual(features); }); - }); - test('should return the number of refs', async () => { - const res = await computeStorybookMetadata({ - packageJson: packageJsonMock, - mainConfig: { - ...mainJsMock, - refs: { - a: { title: '', url: '' }, - b: { title: '', url: '' }, - }, - }, + test('should infer builder and renderer from framework package.json', async () => { + expect( + await computeStorybookMetadata({ + packageJson: packageJsonMock, + mainConfig: { + ...mainJsMock, + framework: '@storybook/react-vite', + }, + }) + ).toMatchObject({ + framework: { name: '@storybook/react-vite' }, + renderer: '@storybook/react', + builder: '@storybook/builder-vite', + }); }); - expect(res.refCount).toEqual(2); - }); - test.each(Object.entries(metaFrameworks))( - 'should detect the supported metaframework: %s', - async (metaFramework, name) => { + test('should return the number of refs', async () => { const res = await computeStorybookMetadata({ - packageJson: { - ...packageJsonMock, - dependencies: { - [metaFramework]: 'x.x.x', + packageJson: packageJsonMock, + mainConfig: { + ...mainJsMock, + refs: { + a: { title: '', url: '' }, + b: { title: '', url: '' }, }, }, - mainConfig: mainJsMock, }); - expect(res.metaFramework).toEqual({ - name, - packageName: metaFramework, - version: 'x.x.x', - }); - } - ); + expect(res.refCount).toEqual(2); + }); + + test.each(Object.entries(metaFrameworks))( + 'should detect the supported metaframework: %s', + async (metaFramework, name) => { + const res = await computeStorybookMetadata({ + packageJson: { + ...packageJsonMock, + dependencies: { + [metaFramework]: 'x.x.x', + }, + } as PackageJson, + mainConfig: mainJsMock, + }); + expect(res.metaFramework).toEqual({ + name, + packageName: metaFramework, + version: 'x.x.x', + }); + } + ); + }); });