Skip to content

Commit

Permalink
Merge pull request #11368 from storybookjs/9773-inconsistent-paths
Browse files Browse the repository at this point in the history
Core: Consistent file paths for locally-defined addons
  • Loading branch information
shilman authored Jul 6, 2020
2 parents 5a9f875 + 357a45e commit 4ea75cb
Show file tree
Hide file tree
Showing 6 changed files with 124 additions and 86 deletions.
19 changes: 19 additions & 0 deletions MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
- [Story Store immutable outside of configuration](#story-store-immutable-outside-of-configuration)
- [Improved story source handling](#improved-story-source-handling)
- [6.0 Addon API changes](#60-addon-api-changes)
- [Consistent local addon paths in main.js](#consistent-local-addon-paths-in-mainjs)
- [Deprecated setAddon](#deprecated-setaddon)
- [Actions addon uses parameters](#actions-addon-uses-parameters)
- [Removed action decorator APIs](#removed-action-decorator-apis)
Expand Down Expand Up @@ -539,6 +540,24 @@ The MDX analog:

### 6.0 Addon API changes

#### Consistent local addon paths in main.js

If you use `.storybook/main.js` config and have locally-defined addons in your project, you need to update your file paths.

In 5.3, `addons` paths were relative to the project root, which was inconsistent with `stories` paths, which were relative to the `.storybook` folder. In 6.0, addon paths are now relative to the config folder.

So, for example, if you had:

```js
module.exports = { addons: ['./.storybook/my-local-addon/register'] };
```

You'd need to update this to:

```js
module.exports = { addons: ['./my-local-addon/register'] };
```

#### Deprecated setAddon

We've deprecated the `setAddon` method of the `storiesOf` API and plan to remove it in 7.0.
Expand Down
3 changes: 3 additions & 0 deletions examples/cra-ts-kitchen-sink/.storybook/localAddon/preset.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
managerEntries: [],
};
15 changes: 15 additions & 0 deletions examples/cra-ts-kitchen-sink/.storybook/localAddon/register.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import React from 'react';
import addons, { types } from '@storybook/addons';

const ID = 'local-addon';

const LocalAddonPanel = () => <>Local addon</>;

addons.register(ID, (api) =>
addons.add(ID, {
title: ID,
type: types.PANEL,
match: () => true,
render: ({ active, key }) => (active ? <LocalAddonPanel key={key} /> : null),
})
);
2 changes: 2 additions & 0 deletions examples/cra-ts-kitchen-sink/.storybook/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ module.exports = {
'@storybook/addon-actions',
'@storybook/addon-links',
'@storybook/addon-a11y',
'./localAddon/register.tsx',
'./localAddon/preset.ts',
],
webpackFinal: (config: Configuration) => {
// add monorepo root as a valid directory to import modules from
Expand Down
62 changes: 22 additions & 40 deletions lib/core/src/server/presets.js
Original file line number Diff line number Diff line change
@@ -1,34 +1,11 @@
import dedent from 'ts-dedent';
import { join } from 'path';
import { logger } from '@storybook/node-logger';
import fs from 'fs';
import { resolveFile } from './utils/resolve-file';
import resolveFrom from 'resolve-from';

const isObject = (val) => val != null && typeof val === 'object' && Array.isArray(val) === false;
const isFunction = (val) => typeof val === 'function';

// Copied out of parse-package-name
// '@storybook/addon-actions/register' => ( name: '@storybook/addon-actions', path: '/register', version: '' )
const RE_SCOPED = /^(@[^/]+\/[^/@]+)(?:\/([^@]+))?(?:@([\s\S]+))?/;
const RE_NORMAL = /^([^/@]+)(?:\/([^@]+))?(?:@([\s\S]+))?/;
function parsePackageName(input) {
if (typeof input !== 'string') {
throw new TypeError('Expected a string');
}

const matched = input.startsWith('@') ? input.match(RE_SCOPED) : input.match(RE_NORMAL);

if (!matched) {
throw new Error(`[parse-package-name] "${input}" is not a valid string`);
}

return {
name: matched[1],
path: matched[2] || '',
version: matched[3] || '',
};
}

const resolvePresetFunction = (input, presetOptions, storybookOptions) => {
if (isFunction(input)) {
return input({ ...storybookOptions, ...presetOptions });
Expand All @@ -40,8 +17,6 @@ const resolvePresetFunction = (input, presetOptions, storybookOptions) => {
return [];
};

const isLocalFileImport = (packageName) => fs.existsSync(packageName);

/**
* Parse an addon into either a managerEntries or a preset. Throw on invalid input.
*
Expand All @@ -58,34 +33,37 @@ const isLocalFileImport = (packageName) => fs.existsSync(packageName);
* - { name: '@storybook/addon-docs(/preset)?', options: { ... } }
* => { type: 'presets', item: { name: '@storybook/addon-docs/preset', options } }
*/
export const resolveAddonName = (name) => {
export const resolveAddonName = (configDir, name) => {
let path;
if (isLocalFileImport(name)) {

if (name.startsWith('.')) {
path = resolveFrom(configDir, name);
} else if (name.startsWith('/')) {
path = name;
} else if (name.match(/\/(preset|register(-panel)?)(\.(js|ts|tsx|jsx))?$/)) {
path = name;
} else {
({ path } = parsePackageName(name));
}

// when user provides full path, we don't need to do anything
if (path) {
return {
name,
name: path,
// Accept `register`, `register.js`, `require.resolve('foo/register'), `register-panel`
type: path.match(/register(-panel)?(.(js|ts|tsx|jsx))?$/) ? 'managerEntries' : 'presets',
type: path.match(/register(-panel)?(\.(js|ts|tsx|jsx))?$/) ? 'managerEntries' : 'presets',
};
}

try {
return {
name: resolveFile(join(name, 'preset')),
name: resolveFrom(configDir, join(name, 'preset')),
type: 'presets',
};
// eslint-disable-next-line no-empty
} catch (err) {}

try {
return {
name: resolveFile(join(name, 'register')),
name: resolveFrom(configDir, join(name, 'register')),
type: 'managerEntries',
};
// eslint-disable-next-line no-empty
Expand All @@ -94,21 +72,21 @@ export const resolveAddonName = (name) => {
return { name, type: 'presets' };
};

export const map = (item) => {
const map = ({ configDir }) => (item) => {
try {
if (isObject(item)) {
const { name } = resolveAddonName(item.name);
const { name } = resolveAddonName(configDir, item.name);
return { ...item, name };
}
const { name, type } = resolveAddonName(item);
const { name, type } = resolveAddonName(configDir, item);
if (type === 'managerEntries') {
return {
name: `${name}_additionalManagerEntries`,
type,
managerEntries: [name],
};
}
return resolveAddonName(name);
return resolveAddonName(configDir, name);
} catch (err) {
logger.error(
`Addon value should end in /register OR it should be a valid preset https://storybook.js.org/docs/presets/introduction/\n${item}`
Expand Down Expand Up @@ -162,7 +140,11 @@ export function loadPreset(input, level, storybookOptions) {

return [
...loadPresets([...subPresets], level + 1, storybookOptions),
...loadPresets([...subAddons.map(map)].filter(Boolean), level + 1, storybookOptions),
...loadPresets(
[...subAddons.map(map(storybookOptions))].filter(Boolean),
level + 1,
storybookOptions
),
{
name,
preset: rest,
Expand Down Expand Up @@ -241,7 +223,7 @@ function applyPresets(presets, extension, config, args, storybookOptions) {
}, presetResult);
}

function getPresets(presets, storybookOptions) {
function getPresets(presets, storybookOptions = {}) {
const loadedPresets = loadPresets(presets, 0, storybookOptions);

return {
Expand Down
109 changes: 63 additions & 46 deletions lib/core/src/server/presets.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,25 +17,27 @@ jest.mock('@storybook/node-logger', () => ({
},
}));

jest.mock('./utils/resolve-file', () => ({
resolveFile: (name) => {
const KNOWN_FILES = [
'@storybook/addon-actions/register',
'@storybook/addon-docs',
'@storybook/addon-docs/preset',
'@storybook/addon-knobs',
'@storybook/addon-notes/register-panel',
'@storybook/preset-typescript',
'addon-bar/preset.js',
'addon-baz/register.js',
'addon-foo/register.js',
];
if (KNOWN_FILES.includes(name)) {
return name;
}
throw new Error(`Cannot find module '${name}'`);
},
}));
jest.mock('resolve-from', () => (l, name) => {
const KNOWN_FILES = [
'@storybook/addon-actions/register',
'./local/preset',
'./local/addons',
'/absolute/preset',
'/absolute/addons',
'@storybook/addon-docs/preset',
'@storybook/addon-knobs/register',
'@storybook/addon-notes/register-panel',
'@storybook/preset-typescript',
'addon-bar/preset.js',
'addon-baz/register.js',
'addon-foo/register.js',
];

if (KNOWN_FILES.includes(name)) {
return name;
}
throw new Error(`cannot resolve ${name}`);
});

describe('presets', () => {
it('does not throw when there is no preset file', async () => {
Expand Down Expand Up @@ -120,7 +122,7 @@ describe('presets', () => {
});

const getPresets = jest.requireActual('./presets').default;
const presets = wrapPreset(getPresets(['preset-foo', 'preset-bar']));
const presets = wrapPreset(getPresets(['preset-foo', 'preset-bar'], {}));

async function testPresets() {
await presets.webpack();
Expand Down Expand Up @@ -341,83 +343,98 @@ describe('resolveAddonName', () => {
const { resolveAddonName } = jest.requireActual('./presets');

it('should resolve packages with metadata (relative path)', () => {
expect(resolveAddonName('@storybook/addon-docs')).toEqual({
name: '@storybook/addon-docs/preset',
mockPreset('./local/preset', {
presets: [],
});
expect(resolveAddonName({}, './local/preset')).toEqual({
name: './local/preset',
type: 'presets',
});
});

it('should resolve packages with metadata (absolute path)', () => {
expect(resolveAddonName('@storybook/addon-knobs')).toEqual({
name: '@storybook/addon-knobs',
mockPreset('/absolute/preset', {
presets: [],
});
expect(resolveAddonName({}, '/absolute/preset')).toEqual({
name: '/absolute/preset',
type: 'presets',
});
});

it('should resolve packages without metadata', () => {
expect(resolveAddonName('@storybook/preset-create-react-app')).toEqual({
expect(resolveAddonName({}, '@storybook/preset-create-react-app')).toEqual({
name: '@storybook/preset-create-react-app',
type: 'presets',
});
});

it('should resolve managerEntries', () => {
expect(resolveAddonName('@storybook/addon-actions/register')).toEqual({
expect(resolveAddonName({}, '@storybook/addon-actions/register')).toEqual({
name: '@storybook/addon-actions/register',
type: 'managerEntries',
});
});

it('should resolve presets', () => {
expect(resolveAddonName('@storybook/addon-docs')).toEqual({
expect(resolveAddonName({}, '@storybook/addon-docs')).toEqual({
name: '@storybook/addon-docs/preset',
type: 'presets',
});
});

it('should resolve preset packages', () => {
expect(resolveAddonName('@storybook/addon-essentials')).toEqual({
expect(resolveAddonName({}, '@storybook/addon-essentials')).toEqual({
name: '@storybook/addon-essentials',
type: 'presets',
});
});

it('should error on invalid inputs', () => {
expect(() => resolveAddonName(null)).toThrow();
expect(() => resolveAddonName({}, null)).toThrow();
});
});

describe('loadPreset', () => {
const { loadPreset } = jest.requireActual('./presets');

mockPreset('@storybook/preset-typescript', {});
mockPreset('@storybook/addon-docs', {});
mockPreset('@storybook/addon-docs/preset', {});
mockPreset('@storybook/addon-actions/register', {});
mockPreset('addon-foo/register.js', {});
mockPreset('addon-bar/preset.js', {});
mockPreset('addon-bar/preset', {});
mockPreset('addon-baz/register.js', {});
mockPreset('@storybook/addon-notes/register-panel', {});

const { loadPreset } = jest.requireActual('./presets');

it('should resolve all addons & presets in correct order', () => {
const loaded = loadPreset({
type: 'managerEntries',
name: '',
presets: ['@storybook/preset-typescript'],
addons: [
'@storybook/addon-docs',
'@storybook/addon-actions/register',
'addon-foo/register.js',
'addon-bar',
'addon-baz/register.tsx',
'@storybook/addon-notes/register-panel',
],
});
const loaded = loadPreset(
{
name: '',
type: 'managerEntries',
presets: ['@storybook/preset-typescript'],
addons: [
'@storybook/addon-docs',
'@storybook/addon-actions/register',
'addon-foo/register.js',
'addon-bar',
'addon-baz/register.tsx',
'@storybook/addon-notes/register-panel',
],
},
0,
{}
);
expect(loaded).toEqual([
{
name: '@storybook/preset-typescript',
options: {},
preset: {},
},
{
name: '@storybook/addon-docs/preset',
options: {},
preset: {},
},
{
name: '@storybook/addon-actions/register_additionalManagerEntries',
options: {},
Expand Down

0 comments on commit 4ea75cb

Please sign in to comment.