-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
feat: add defineMeta
#25069
feat: add defineMeta
#25069
Conversation
Thank you for your contribution to Storybook with the defineMeta utility. Before moving forward, there are several considerations and steps we should address: CSF files need to remain statically analyzable. The addition of helper functions like defineMeta could potentially obscure static analysis. It's important to ensure that any such utility does not interfere with static analysis required by Storybook's tools. There seems to be a concern about the integration between defineMeta and a potential defineStory function. The issue #18228 refers to having a declarative way to define stories which defineMeta alone does not provide. How the two can be combined while retaining type inference needs to be discussed further. Given the potential implications for CSF's design and static analysis capabilities, I recommend formalizing your proposal through the RFC (Request for Comments) process as your PR raises questions that warrant broader discussion with the core team and community. Writing an RFC in the GitHub Discussions will allow for collaborative exploration of how defineStory and defineMeta can and should be used together. It also provides a space for addressing any concerns related to static analyzability and integration with existing and future Storybook APIs. For now, I will close this PR. Please proceed with the following steps:
|
Thanks for the comments @valentinpalkovic. This PR considers only I'll look into about writing up a RFC about |
@AriPerkkio But it was rejected in favour of using I want to propose this style for CSF4. Ideally we would have something that makes sure the relationship between meta and story is understood out of the box, without much type hassling. At this moment, we recommend: import type { Meta, StoryObj } from '@storybook/react';
import { Button } from './Button';
const meta = {
component: Button,
} satisfies Meta<typeof Button>;
export default meta;
type Story = StoryObj<typeof meta>;
export const Primary: Story = {
args: {},
}; With your proposal, we can clean this up as: import type { Meta, StoryObj } from '@storybook/react';
import { Button } from './Button';
const meta = defineMeta({
component: Button,
});
export default meta;
type Story = StoryObj<typeof meta>;
export const Primary: Story = {
args: {},
}; Which is an improvement, but if we are gonna recommend factories, I would like to go all the way: import type { Meta, StoryObj } from '@storybook/react';
import { Button } from './Button';
const { meta, defineStory } = defineMeta({
component: Button,
});
export default meta;
export const Primary = defineStory({
args: {},
}); This looks a bit cleaner to me. And this would also allow us to get rid of type annotations for stories. However, for CSF4, we might even go further. As one problem with our TS support is that we can not infer the parameters that are added by addons. One other idea for CSF4 is to make the relationship between addons/preview.ts and stories completely explicit (even on runtime). Instead of main.ts and preview.ts files, I am thinking about renaming them as:
The main difference is that In that line, we could have something like: // stories.config.ts
import { defineConfig } from '@storybook/react';
import { mswLoader } from 'addon-msw';
export default defineConfig({
loaders: [
mswLoader,
],
decorators: [...],
}) // Card.stories.ts
import config from '~/stories.config.ts'
import { Card } from './Card';
import { setSystemTime } from '@storybook/test';
// maybe just call it extend, as in some sense meta is just extending the config of preview
const { defineStory } = config.extend({
component: Card,
args: {
disabled: false, // disabled type inferred from Card
},
argTypes: {
// Another idea, is to make $-args, not end up in the Component, but can be used for other purposes
$date: 'date',
},
// we are considering beforeRender for CSF4
beforeRender: ({ args }) => {
// TS knows about this $date, because it is inferred from argTypes
if (args.$date) setSystemTime(args.$date)
},
})
// we might not need to export meta explicitly in CSF4, so jeej
export const Default = defineStory({
args: {
$date: new Date(2020, 11, 24) // special christmas UI!
},
parameters: [
msw: [
http.get('/endpoint', () => Response.json({...})),
// parameters.msw type inferred all the way up from mswLoader!
],
],
}) Btw, these are all just my own thoughts and not yet accepted in any way. |
Those are great improvement ideas for Typescript support. I really like the proposed I myself don't usually type stories at all with @kasperpeulen do you think I should still open RFC for |
I think ideas for CSF4 are definitely welcome to be put into a RFC. Maybe @kasperpeulen and you might collaborate to create one :) |
@AriPerkkio Thanks so much for this PR. I think we're all in agreement that this is the way to go. My primary concern is not about the whether these type-safe factory functions are a good idea or whether they are statically analyzable, but when and how to introduce them. Every time we change the story format it is hugely disruptive for our users, who have written literally millions of stories in the old format. Even though most of the stories can be automatically transformed from CSF 1 to 2 and 2 to 3 with codemods, is still something new to learn for users. Also, blog posts and docs go out of date. Therefore, for CSF4, I hope we can introduce a few different improvements at once, similar to some of the stuff @kasperpeulen has proposed above. In addition to everything said here, next year we'll be focusing on making it easy to build entire pages in Storybook. I suspect in the course of that, we'll pick up a few more CSF4 constructs. If you and Kasper create an RFC for these factory functions, we can discuss there. I'm confident we can figure out a good way to release this experimentally in 8.x so that early adopters can use it while the entirety of CSF4 takes shape. |
I have now opened RFC considering |
Closes #18228
What I did
Adds new helper utility
defineMeta
.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
yarn task --task sandbox --start-from auto --template react-vite/default-ts
cd sandbox/react-vite-default-ts
sandbox/react-vite-default-ts/src/stories/Button.stories.ts
in your code editoryarn storybook
and verify Button stories work in browsercsf-tools/src/CsfFile.ts
and see how stories no longer work.Documentation
MIGRATION.MD
Should I add documentation to
docs/writing-stories/typescript.md
? Maybe under the## Using 'satisfies' for better type safety
?Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal
,ci:merged
orci:daily
GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli/src/sandbox-templates.ts
Make sure this PR contains one of the labels below:
Available labels
bug
: Internal changes that fixes incorrect behavior.maintenance
: User-facing maintenance tasks.dependencies
: Upgrading (sometimes downgrading) dependencies.build
: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup
: Minor cleanup style change. Will not show up in release changelog.documentation
: Documentation only changes. Will not show up in release changelog.feature request
: Introducing a new feature.BREAKING CHANGE
: Changes that break compatibility in some way with current major version.other
: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/core
team here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>