Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Codemods and refactorings #30292

Merged
merged 27 commits into from
Jan 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
f1a2c53
Csf Tools: Support CSF4 in enrichCsf for source extraction
yannbf Jan 13, 2025
6c27056
Apply suggestions from code review
yannbf Jan 13, 2025
659930e
Use csf-tools in csf factory codemod
yannbf Jan 15, 2025
7db0050
Add defineConfig helper for main.js
yannbf Jan 14, 2025
ebe6bf9
move defineConfig to its own import path
yannbf Jan 14, 2025
df000bb
fix internal type import
yannbf Jan 17, 2025
5ac85ef
rename the import and function
yannbf Jan 17, 2025
778405d
Move csf factory codemod to automigrations
yannbf Jan 16, 2025
e68b8f9
address review comments
yannbf Jan 16, 2025
1f79546
refactor and use automigrate instead of codemod in sandbox
yannbf Jan 17, 2025
b0972bc
remove entrypoint
yannbf Jan 17, 2025
325a554
only apply codemod to starter stories in sandbox, rename to csf-facto…
yannbf Jan 17, 2025
9df03e6
fix angular and ember build
yannbf Jan 17, 2025
37811bb
Merge pull request #30282 from storybookjs/yann/move-csf-factory-to-a…
yannbf Jan 17, 2025
503f883
Merge pull request #30250 from storybookjs/yann/csf-factories-extra
yannbf Jan 17, 2025
d4e3695
ConfigFile: Support pnp wrapped names
yannbf Jan 17, 2025
6648198
add codemod for main/preview in factory format
yannbf Jan 17, 2025
211fb4b
add storybook config + addon sync codemod
yannbf Jan 20, 2025
310ebf2
sync main and preview in sb add
yannbf Jan 20, 2025
f38a9f6
Merge pull request #30312 from storybookjs/yann/csf-config-factory-co…
yannbf Jan 20, 2025
5c3752a
move defineMainConfig definition to the index level
yannbf Jan 20, 2025
04d9f58
modify preview/main config format in codemods/csf file
yannbf Jan 20, 2025
71fefda
add fixes, fix types, fix tests
yannbf Jan 20, 2025
69941ca
fix portable stories test
yannbf Jan 20, 2025
5b4cd43
fix build
yannbf Jan 20, 2025
80c80ac
fix lint
yannbf Jan 20, 2025
2ec88ee
update snapshots
yannbf Jan 21, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions code/.storybook/main.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { join } from 'node:path';

import type { StorybookConfig } from '../frameworks/react-vite';
import { defineMain } from '../frameworks/react-vite';

const componentsPath = join(__dirname, '../core/src/components');
const managerApiPath = join(__dirname, '../core/src/manager-api');
const imageContextPath = join(__dirname, '..//frameworks/nextjs/src/image-context.ts');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: extra forward slash in path '..//frameworks/nextjs/src/image-context.ts'


const config: StorybookConfig = {
const config = defineMain({
stories: [
'./*.stories.@(js|jsx|ts|tsx)',
{
Expand Down Expand Up @@ -169,6 +169,6 @@ const config: StorybookConfig = {
} satisfies typeof viteConfig);
},
// logLevel: 'debug',
};
});

export default config;
4 changes: 2 additions & 2 deletions code/.storybook/preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
import { DocsContext } from '@storybook/blocks';
import { global } from '@storybook/global';
import type { Decorator, Loader, ReactRenderer } from '@storybook/react';
import { defineConfig } from '@storybook/react/preview';
import { definePreview } from '@storybook/react/preview';

// TODO add empty preview
// import * as storysource from '@storybook/addon-storysource';
Expand Down Expand Up @@ -375,7 +375,7 @@ const parameters = {
},
};

export const config = defineConfig({
export const config = definePreview({
addons: [addonThemes, essentials, a11y, addonsPreview, templatePreview],
parameters,
decorators,
Expand Down
57 changes: 41 additions & 16 deletions code/core/src/csf-tools/ConfigFile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,13 +245,25 @@ describe('ConfigFile', () => {
});

describe('factory config', () => {
it('parses correctly', () => {
const source = dedent`
import { definePreview } from '@storybook/react-vite/preview';

const config = definePreview({
framework: 'foo',
});
export default config;
`;
const config = loadConfig(source).parse();
expect(config.getNameFromPath(['framework'])).toEqual('foo');
});
it('found scalar', () => {
expect(
getField(
['core', 'builder'],
dedent`
import { defineConfig } from '@storybook/react-vite/preview';
export const foo = defineConfig({ core: { builder: 'webpack5' } });
import { definePreview } from '@storybook/react-vite/preview';
export const foo = definePreview({ core: { builder: 'webpack5' } });
`
)
).toEqual('webpack5');
Expand All @@ -261,9 +273,9 @@ describe('ConfigFile', () => {
getField(
['tags'],
dedent`
import { defineConfig } from '@storybook/react-vite/preview';
import { definePreview } from '@storybook/react-vite/preview';
const parameters = {};
export const config = defineConfig({
export const config = definePreview({
parameters,
tags: ['test', 'vitest', '!a11ytest'],
});
Expand Down Expand Up @@ -516,15 +528,15 @@ describe('ConfigFile', () => {
['core', 'builder'],
'webpack5',
dedent`
import { defineConfig } from '@storybook/react-vite/preview';
export const foo = defineConfig({
import { definePreview } from '@storybook/react-vite/preview';
export const foo = definePreview({
addons: [],
});
`
)
).toMatchInlineSnapshot(`
import { defineConfig } from '@storybook/react-vite/preview';
export const foo = defineConfig({
import { definePreview } from '@storybook/react-vite/preview';
export const foo = definePreview({
addons: [],

core: {
Expand All @@ -539,15 +551,15 @@ describe('ConfigFile', () => {
['core', 'builder'],
'webpack5',
dedent`
import { defineConfig } from '@storybook/react-vite/preview';
export const foo = defineConfig({
import { definePreview } from '@storybook/react-vite/preview';
export const foo = definePreview({
core: { foo: 'bar' },
});
`
)
).toMatchInlineSnapshot(`
import { defineConfig } from '@storybook/react-vite/preview';
export const foo = defineConfig({
import { definePreview } from '@storybook/react-vite/preview';
export const foo = definePreview({
core: {
foo: 'bar',
builder: 'webpack5'
Expand All @@ -561,15 +573,15 @@ describe('ConfigFile', () => {
['core', 'builder'],
'webpack5',
dedent`
import { defineConfig } from '@storybook/react-vite/preview';
export const foo = defineConfig({
import { definePreview } from '@storybook/react-vite/preview';
export const foo = definePreview({
core: { builder: 'webpack4' },
});
`
)
).toMatchInlineSnapshot(`
import { defineConfig } from '@storybook/react-vite/preview';
export const foo = defineConfig({
import { definePreview } from '@storybook/react-vite/preview';
export const foo = definePreview({
core: { builder: 'webpack5' },
});
`);
Expand Down Expand Up @@ -1017,6 +1029,19 @@ describe('ConfigFile', () => {
expect(config.getNameFromPath(['otherField'])).toEqual('foo');
});

it(`supports pnp wrapped names`, () => {
const source = dedent`
import type { StorybookConfig } from '@storybook/react-webpack5';

const config: StorybookConfig = {
framework: getAbsolutePath('foo'),
}
export default config;
`;
const config = loadConfig(source).parse();
expect(config.getNameFromPath(['framework'])).toEqual('foo');
});

it(`returns undefined when accessing a field that does not exist`, () => {
const source = dedent`
import type { StorybookConfig } from '@storybook/react-webpack5';
Expand Down
19 changes: 8 additions & 11 deletions code/core/src/csf-tools/ConfigFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,8 @@ const getCsfParsingErrorMessage = ({
foundType: string | undefined;
node: any | undefined;
}) => {
let nodeInfo = '';
if (node) {
try {
nodeInfo = JSON.stringify(node);
} catch (e) {
//
}
}

return dedent`
CSF Parsing error: Expected '${expectedType}' but found '${foundType}' instead in '${node?.type}'.
${nodeInfo}
`;
};

Expand Down Expand Up @@ -215,6 +205,11 @@ export class ConfigFile {

decl = unwrap(decl);

// csf factory
if (t.isCallExpression(decl) && t.isObjectExpression(decl.arguments[0])) {
decl = decl.arguments[0];
}

if (t.isObjectExpression(decl)) {
self._parseExportsObject(decl);
} else {
Expand Down Expand Up @@ -323,7 +318,7 @@ export class ConfigFile {
enter: ({ node }) => {
if (
t.isIdentifier(node.callee) &&
node.callee.name === 'defineConfig' &&
node.callee.name === 'definePreview' &&
node.arguments.length === 1 &&
t.isObjectExpression(node.arguments[0])
) {
Expand Down Expand Up @@ -499,6 +494,8 @@ export class ConfigFile {
value = prop.value.value;
}
});
} else if (t.isCallExpression(node)) {
value = this._getPnpWrappedValue(node);
}

if (!value) {
Expand Down
12 changes: 11 additions & 1 deletion code/core/src/csf-tools/CsfFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { readFile, writeFile } from 'node:fs/promises';
import {
BabelFileClass,
type GeneratorOptions,
type NodePath,
type RecastOptions,
babelParse,
generate,
Expand Down Expand Up @@ -255,10 +256,14 @@ export class CsfFile {

_storyExports: Record<string, t.VariableDeclarator | t.FunctionDeclaration> = {};

_storyPaths: Record<string, NodePath<t.ExportNamedDeclaration>> = {};

_metaStatement: t.Statement | undefined;

_metaNode: t.Expression | undefined;

_metaPath: NodePath<t.ExportDefaultDeclaration> | undefined;

_metaVariableName: string | undefined;

_metaIsFactory: boolean | undefined;
Expand Down Expand Up @@ -466,10 +471,13 @@ export class CsfFile {
self._options.fileName
);
}

self._metaPath = path;
},
},
ExportNamedDeclaration: {
enter({ node, parent }) {
enter(path) {
const { node, parent } = path;
let declarations;
if (t.isVariableDeclaration(node.declaration)) {
declarations = node.declaration.declarations.filter((d) => t.isVariableDeclarator(d));
Expand All @@ -487,6 +495,7 @@ export class CsfFile {
return;
}
self._storyExports[exportName] = decl;
self._storyPaths[exportName] = path;
self._storyStatements[exportName] = node;
let name = storyNameFromExport(exportName);
if (self._storyAnnotations[exportName]) {
Expand Down Expand Up @@ -611,6 +620,7 @@ export class CsfFile {
} else {
self._storyAnnotations[exportName] = {};
self._storyStatements[exportName] = decl;
self._storyPaths[exportName] = path;
self._stories[exportName] = {
id: 'FIXME',
name: exportName,
Expand Down
45 changes: 45 additions & 0 deletions code/core/src/csf-tools/enrichCsf.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,51 @@ describe('enrichCsf', () => {
};
`);
});
it('csf factories', () => {
expect(
enrich(
dedent`
// compiled code
import {config} from "/.storybook/preview.ts";
const meta = config.meta({
args: {
label: "Hello world!"
}
});
export const Story = meta.story({});
`,
dedent`
// original code
import {config} from "#.storybook/preview.ts";
const meta = config.meta({
args: {
label: "Hello world!"
}
});
export const Story = meta.story({});
`
)
).toMatchInlineSnapshot(`
// compiled code
import { config } from "/.storybook/preview.ts";
const meta = config.meta({
args: {
label: "Hello world!"
}
});
export const Story = meta.story({});
Story.annotations.parameters = {
...Story.annotations.parameters,
docs: {
...Story.annotations.parameters?.docs,
source: {
originalSource: "meta.story({})",
...Story.annotations.parameters?.docs?.source
}
}
};
`);
});
it('multiple stories', () => {
expect(
enrich(
Expand Down
11 changes: 10 additions & 1 deletion code/core/src/csf-tools/enrichCsf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,20 @@ export const enrichCsfStory = (
options?: EnrichCsfOptions
) => {
const storyExport = csfSource.getStoryExport(key);
const isCsfFactory =
t.isCallExpression(storyExport) &&
t.isMemberExpression(storyExport.callee) &&
t.isIdentifier(storyExport.callee.object) &&
storyExport.callee.object.name === 'meta';
Comment on lines +18 to +22
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: This factory detection could miss edge cases where meta is imported under a different name. Consider making this more robust by checking the call signature rather than just the name.

const source = !options?.disableSource && extractSource(storyExport);
const description =
!options?.disableDescription && extractDescription(csfSource._storyStatements[key]);
const parameters = [];
const originalParameters = t.memberExpression(t.identifier(key), t.identifier('parameters'));
// in csf 1/2/3 use Story.parameters; CSF factories use Story.annotations.parameters
const baseStoryObject = isCsfFactory
? t.memberExpression(t.identifier(key), t.identifier('annotations'))
: t.identifier(key);
const originalParameters = t.memberExpression(baseStoryObject, t.identifier('parameters'));
parameters.push(t.spreadElement(originalParameters));
const optionalDocs = t.optionalMemberExpression(
originalParameters,
Expand Down
10 changes: 10 additions & 0 deletions code/frameworks/angular/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,16 @@
"url": "https://opencollective.com/storybook"
},
"license": "MIT",
"exports": {
".": {
"types": "./dist/index.d.ts",
"node": "./dist/index.js",
"import": "./dist/index.mjs",
"require": "./dist/index.js"
},
"./preset": "./preset.js",
"./package.json": "./package.json"
},
"main": "dist/index.js",
"module": "dist/index.mjs",
"types": "dist/index.d.ts",
Expand Down
5 changes: 5 additions & 0 deletions code/frameworks/angular/src/defineMainConfig.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { StorybookConfig } from './types';

export function defineMain(config: StorybookConfig) {
return config;
}
1 change: 1 addition & 0 deletions code/frameworks/angular/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export * from './client/index';
export * from './types';
export * from './defineMainConfig';

/*
* ATTENTION:
Expand Down
10 changes: 10 additions & 0 deletions code/frameworks/ember/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,16 @@
"url": "https://opencollective.com/storybook"
},
"license": "MIT",
"exports": {
".": {
"types": "./dist/index.d.ts",
"node": "./dist/index.js",
"import": "./dist/index.mjs",
"require": "./dist/index.js"
},
"./preset": "./preset.js",
"./package.json": "./package.json"
},
"main": "dist/index.js",
"module": "dist/index.mjs",
"types": "dist/index.d.ts",
Expand Down
5 changes: 5 additions & 0 deletions code/frameworks/ember/src/defineMainConfig.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import type { StorybookConfig } from './types';

export function defineMain(config: StorybookConfig) {
return config;
}
1 change: 1 addition & 0 deletions code/frameworks/ember/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/// <reference types="webpack-env" />

export * from './types';
export * from './defineMainConfig';

// optimization: stop HMR propagation in webpack

Expand Down
Loading
Loading