Skip to content

Commit

Permalink
improve framework field validation
Browse files Browse the repository at this point in the history
  • Loading branch information
yannbf committed Feb 28, 2023
1 parent 475b6d3 commit 5f44f0a
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 13 deletions.
32 changes: 32 additions & 0 deletions code/lib/core-common/src/utils/validate-config.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { validateFrameworkName } from './validate-config';

describe('validateFrameworkName', () => {
afterEach(jest.resetAllMocks);
it('should throw if name is undefined', () => {
expect(() => validateFrameworkName(undefined)).toThrow();
});

it('should throw if name is a renderer', () => {
expect(() => validateFrameworkName('react')).toThrow();
expect(() => validateFrameworkName('@storybook/react')).toThrow();
});

it('should not throw if framework is a known framework', () => {
expect(() => validateFrameworkName('@storybook/react-vite')).not.toThrow();
});

it('should not throw if framework is unknown (community) but can be resolved', () => {
// mock require.resolve to return a value
jest.spyOn(require, 'resolve').mockReturnValue('some-community-framework');
expect(() => validateFrameworkName('some-community-framework')).toThrow();
});

it('should throw if framework is unknown and cannot be resolved', () => {
// mock require.resolve to fail
jest.spyOn(require, 'resolve').mockImplementation(() => {
throw new Error('Cannot resolve');
});

expect(() => validateFrameworkName('foo')).toThrow();
});
});
42 changes: 35 additions & 7 deletions code/lib/core-common/src/utils/validate-config.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,47 @@
import { dedent } from 'ts-dedent';
import { frameworkPackages } from './get-storybook-info';

const renderers = ['html', 'preact', 'react', 'server', 'svelte', 'vue', 'vue3', 'web-components'];

const rendererNames = [...renderers, ...renderers.map((renderer) => `@storybook/${renderer}`)];

// Checks that the framework name is not a renderer
export function validateFrameworkName(frameworkName: string) {
export function validateFrameworkName(frameworkName: string | undefined) {
const automigrateMessage = `Please run 'npx storybook@next automigrate' to automatically fix your config.
See the migration guide for more information:
https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#new-framework-api\n`;

// Throw error if there is no framework field
// TODO: maybe this error should not be thrown if we allow empty Storybooks that only use "refs" for composition
if (!frameworkName) {
throw new Error(dedent`
Could not find a 'framework' field in Storybook config.
${automigrateMessage}
`);
}

// Account for legacy scenario where the framework was referring to a renderer
if (rendererNames.includes(frameworkName)) {
throw new Error(dedent`
Invalid value of ${frameworkName} in the 'framework' field of Storybook config.
Invalid value of '${frameworkName}' in the 'framework' field of Storybook config.
Please run 'npx sb@next automigrate'
${automigrateMessage}
`);
}

// If we know about the framework, we don't need to validate it
if (Object.keys(frameworkPackages).includes(frameworkName)) {
return;
}

// If it's not a known framework, we need to validate that it's a valid package at least
try {
require.resolve(frameworkName);
} catch (err) {
throw new Error(dedent`
Could not evaluate the ${frameworkName} package from the 'framework' field of Storybook config.
See the v7 Migration guide for more information:
https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#framework-field-mandatory
`);
Are you sure it's a valid package and is installed?`);
}
}
7 changes: 1 addition & 6 deletions code/lib/core-server/src/build-dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import { global } from '@storybook/global';
import { telemetry } from '@storybook/telemetry';

import { join, resolve } from 'path';
import { logger } from '@storybook/node-logger';
import { storybookDevServer } from './dev-server';
import { getReleaseNotesData, getReleaseNotesFailedState } from './utils/release-notes';
import { outputStats } from './utils/output-stats';
Expand Down Expand Up @@ -73,11 +72,7 @@ export async function buildDevStandalone(
const frameworkName = typeof framework === 'string' ? framework : framework?.name;
validateFrameworkName(frameworkName);

if (frameworkName) {
corePresets.push(join(frameworkName, 'preset'));
} else {
logger.warn(`you have not specified a framework in your ${options.configDir}/main.js`);
}
corePresets.push(join(frameworkName, 'preset'));

// Load first pass: We need to determine the builder
// We need to do this because builders might introduce 'overridePresets' which we need to take into account
Expand Down

0 comments on commit 5f44f0a

Please sign in to comment.