From fd5bd13dcbc4965c5778de2fa653713fb4093bcc Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Thu, 19 Jan 2023 17:57:48 +0100 Subject: [PATCH 1/2] replace getFieldValue with getNameFromPath where applicable - This makes the code safer as it does not evaluate properties, just looks for string literals that identify that property --- code/lib/cli/src/automigrate/fixes/mainjsFramework.ts | 2 +- code/lib/cli/src/automigrate/fixes/missing-babelrc.ts | 11 +++-------- code/lib/cli/src/automigrate/fixes/new-frameworks.ts | 2 +- .../cli/src/automigrate/fixes/sveltekit-framework.ts | 6 ++---- 4 files changed, 7 insertions(+), 14 deletions(-) diff --git a/code/lib/cli/src/automigrate/fixes/mainjsFramework.ts b/code/lib/cli/src/automigrate/fixes/mainjsFramework.ts index b08460602cf8..ad27f0e9bab6 100644 --- a/code/lib/cli/src/automigrate/fixes/mainjsFramework.ts +++ b/code/lib/cli/src/automigrate/fixes/mainjsFramework.ts @@ -38,7 +38,7 @@ export const mainjsFramework: Fix = { } const main = await readConfig(mainConfig); - const currentFramework = main.getFieldValue(['framework']); + const currentFramework = main.getFieldNode(['framework']); const features = main.getFieldValue(['features']); if (currentFramework) return null; diff --git a/code/lib/cli/src/automigrate/fixes/missing-babelrc.ts b/code/lib/cli/src/automigrate/fixes/missing-babelrc.ts index 3e7fdc59f9f8..322c44f14f07 100644 --- a/code/lib/cli/src/automigrate/fixes/missing-babelrc.ts +++ b/code/lib/cli/src/automigrate/fixes/missing-babelrc.ts @@ -46,16 +46,11 @@ export const missingBabelRc: Fix = { const main = await readConfig(mainConfig); - const frameworkField = main.getFieldValue(['framework']); - const frameworkPackage = - typeof frameworkField === 'string' ? frameworkField : frameworkField?.name; + const frameworkPackage = main.getNameFromPath(['framework']); - const addons: any[] = main.getFieldValue(['addons']) || []; + const addons = main.getNamesFromPath(['addons']); - const hasCraPreset = addons.find((addon) => { - const name = typeof addon === 'string' ? addon : addon.name; - return name === '@storybook/preset-create-react-app'; - }); + const hasCraPreset = addons.find((addon) => addon === '@storybook/preset-create-react-app'); if (frameworksThatNeedBabelConfig.includes(frameworkPackage) && !hasCraPreset) { const config = await loadPartialConfigAsync({ diff --git a/code/lib/cli/src/automigrate/fixes/new-frameworks.ts b/code/lib/cli/src/automigrate/fixes/new-frameworks.ts index d6935e92c703..7850eca4aa25 100644 --- a/code/lib/cli/src/automigrate/fixes/new-frameworks.ts +++ b/code/lib/cli/src/automigrate/fixes/new-frameworks.ts @@ -127,7 +127,7 @@ export const newFrameworks: Fix = { // If in the future the eslint plugin has a framework option, using main to extract the framework field will be very useful const main = await readConfig(mainConfig); - const frameworkPackage = main.getFieldValue(['framework']) as keyof typeof packagesMap; + const frameworkPackage = main.getNameFromPath(['framework']) as keyof typeof packagesMap; const builder = main.getFieldValue(['core', 'builder']); if (!frameworkPackage) { diff --git a/code/lib/cli/src/automigrate/fixes/sveltekit-framework.ts b/code/lib/cli/src/automigrate/fixes/sveltekit-framework.ts index 3422da2cfc61..d990ac2ebb4f 100644 --- a/code/lib/cli/src/automigrate/fixes/sveltekit-framework.ts +++ b/code/lib/cli/src/automigrate/fixes/sveltekit-framework.ts @@ -57,9 +57,9 @@ export const sveltekitFramework: Fix = { } const main = await readConfig(mainConfig); - const frameworkConfig = main.getFieldValue(['framework']); + const framework = main.getNameFromPath(['framework']); - if (!frameworkConfig) { + if (!framework) { logger.warn(dedent` ❌ Unable to determine Storybook framework, skipping ${chalk.cyan(fixId)} fix. 🤔 Are you running automigrate from your project directory? @@ -67,8 +67,6 @@ export const sveltekitFramework: Fix = { return null; } - const framework = typeof frameworkConfig === 'string' ? frameworkConfig : frameworkConfig.name; - if (framework === '@storybook/sveltekit') { // already using the new framework return null; From f380b27244bedbd6b65db6ebd3f236be940c7345 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Fri, 20 Jan 2023 10:31:45 +0100 Subject: [PATCH 2/2] small tweaks on babelrc automigration --- .../automigrate/fixes/missing-babelrc.test.ts | 32 +++++++++++++++---- .../src/automigrate/fixes/missing-babelrc.ts | 11 +++++-- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/code/lib/cli/src/automigrate/fixes/missing-babelrc.test.ts b/code/lib/cli/src/automigrate/fixes/missing-babelrc.test.ts index 1f896ed5e8e8..4f706b130fb2 100644 --- a/code/lib/cli/src/automigrate/fixes/missing-babelrc.test.ts +++ b/code/lib/cli/src/automigrate/fixes/missing-babelrc.test.ts @@ -49,18 +49,33 @@ describe('missing-babelrc fix', () => { // different babel extensions await expect( - check({ extraFiles: { '.babelrc': babelContent }, packageJson }) + check({ + extraFiles: { '.babelrc': babelContent }, + packageJson, + main: { framework: '@storybook/react' }, + }) ).resolves.toBeNull(); await expect( - check({ extraFiles: { '.babelrc.json': babelContent }, packageJson }) + check({ + extraFiles: { '.babelrc.json': babelContent }, + packageJson, + main: { framework: '@storybook/react' }, + }) ).resolves.toBeNull(); await expect( - check({ extraFiles: { 'babel.config.json': babelContent }, packageJson }) + check({ + extraFiles: { 'babel.config.json': babelContent }, + packageJson, + main: { framework: '@storybook/react' }, + }) ).resolves.toBeNull(); // babel field in package.json await expect( - check({ packageJson: { ...packageJson, babel: babelContent } }) + check({ + packageJson: { ...packageJson, babel: babelContent }, + main: { framework: '@storybook/react' }, + }) ).resolves.toBeNull(); }); @@ -72,7 +87,9 @@ describe('missing-babelrc fix', () => { }, }; - await expect(check({ packageJson })).resolves.toBeNull(); + await expect( + check({ packageJson, main: { framework: '@storybook/nextjs' } }) + ).resolves.toBeNull(); }); it('skips when using CRA preset', async () => { @@ -84,7 +101,10 @@ describe('missing-babelrc fix', () => { }; await expect( - check({ packageJson, main: { addons: ['@storybook/preset-create-react-app'] } }) + check({ + packageJson, + main: { framework: '@storybook/react', addons: ['@storybook/preset-create-react-app'] }, + }) ).resolves.toBeNull(); }); diff --git a/code/lib/cli/src/automigrate/fixes/missing-babelrc.ts b/code/lib/cli/src/automigrate/fixes/missing-babelrc.ts index 322c44f14f07..a2510410c1d0 100644 --- a/code/lib/cli/src/automigrate/fixes/missing-babelrc.ts +++ b/code/lib/cli/src/automigrate/fixes/missing-babelrc.ts @@ -50,9 +50,14 @@ export const missingBabelRc: Fix = { const addons = main.getNamesFromPath(['addons']); - const hasCraPreset = addons.find((addon) => addon === '@storybook/preset-create-react-app'); - - if (frameworksThatNeedBabelConfig.includes(frameworkPackage) && !hasCraPreset) { + const hasCraPreset = + addons && addons.find((addon) => addon === '@storybook/preset-create-react-app'); + + if ( + frameworkPackage && + frameworksThatNeedBabelConfig.includes(frameworkPackage) && + !hasCraPreset + ) { const config = await loadPartialConfigAsync({ babelrc: true, filename: '__fake__.js', // somehow needed to detect .babelrc.* files