CSF3 react templates for storybook init #19462

kasperpeulen · 2022-10-12T09:41:15Z

Issue:

What I did

Upgrade TS to 4.9
Update react templates for storybook init to CSF3
I included the templates (except for aurelia) and test files in tsc, and fixed all the tsc errors.

Blocked by

Prettier and eslint supporting TS 4.9 (TypeScript 4.9 Support typescript-eslint/typescript-eslint#5688)
Removal of angular examples so that we can upgrade to TS4.9 internally.

How to test

I added some unit tests, but I didn't test it yet in an e2e kind of setup. Should I do this manually?

.vscode/settings.json

kasperpeulen · 2022-10-12T09:43:34Z

code/lib/cli/src/add.ts

@@ -23,6 +22,8 @@ const postinstallAddon = async (addonName: string, isOfficialAddon: boolean) =>
    LEGACY_CONFIGS.forEach((config) => {
      try {
        const codemod = require.resolve(
+          // TODO: fix this before merging
+          // @ts-expect-error getPackageName undefined, so will always go to catch branch


I don't understand this, seems impossible for this code to work if getPackageName is not imported

yeah this code has been broken for a while, I suspect some bad merge from the past removed the import.

kylegach · 2022-10-12T15:54:27Z

code/lib/cli/rendererAssets/react/ts/Button.stories.tsx

-import React from 'react';
-import type { ComponentStoryFn, ComponentMeta } from '@storybook/react';
-
+import type { Meta, StoryObj } from '@storybook/react';
 import { Button } from './Button';

 // More on default export: https://storybook.js.org/docs/react/writing-stories/introduction#default-export


@jonniebigodes — I think this deserves a pass with eyes toward docs/DX. For example:

Now that this comment is placed before const meta and not export default, should the comment say "More on meta". And should all the "default export" references in the docs be changed to "meta"?

Do we need a comment about the default (implicit) render function?

I agree. I think you are right, so I already changed the template to "More on meta"

I think so. Or at least an example with an explicit render function.

@kylegach and @kasperpeulen, regarding the implicit/explicit render function, I would suggest going with one of the examples having an explicit render function, for instance, the Secondary Button story, and add a comment above it, for example, something like:

// Button.stories.js|jsx|ts|tsx // imports and other information goes here /* *👇 CSF 3 render functions are a framework-specific feature to allow you control over how the component renders. * See https://storybook.js.org/docs/7.0/react/api/csf * to learn how to use render functions. */ export const Secondary = { render: (args) => ( <Button {...args} /> ), args: { primary: true, label: 'Button', }, }

And in the others with an implicit render function, we could go with something like:

// Button.stories.js|jsx|ts|tsx // imports and other information goes here // CSF 3 story using an implicit render function export const Primary = { args: { primary: true, label: 'Button', }, }

Regarding the const meta usage instead of the default export a couple of items to address:
1- If this is the intended path for every framework, the TS snippets in use need updating, and I'll follow up with you @kasperpeulen on them.
2- Instead of // More on meta comment instead, we could go with something like

// Button.stories.js|jsx|ts|tsx // imports go here // More on how to set up Storybook stories at: https://storybook.js.org/docs/react/writing-stories/introduction#default-export const meta = { title: 'Example/Button', component: Button, // More on argTypes: https://storybook.js.org/docs/react/api/argtypes argTypes: { backgroundColor: { control: 'color' }, }, } satisfies Meta<typeof Button>; export default meta;

The reasoning behind this is that leaving it only // More on meta , the wording might throw off new users that installing Storybook for the first time and have no knowledge of how it works.

What do you think?

Sounds good to me @jonniebigodes.

I'm still figuring out the other frameworks, I think it will be the intended path for Vue 2/3 and Svelte. Not sure yet about Lit and Angular, as I think we can not improve the type safety there with satisfies. We could still use the same format to keep it consistent.

That wording also sounds good to me, I will leave that decision to you guys :)

kylegach · 2022-10-12T15:55:09Z

code/lib/cli/rendererAssets/react/ts/Button.stories.tsx

+};
+satisfies;
+Meta<typeof Button>;


Is the format here right? I would've expected:

} satisfies Meta<typeof Button>;

Oh wow, I think eslint fix did that :D

So unfixed the fix :P But looks better now :D Eslint doesn't support TS 4.9 yet.

Maybe we just have to upgrade the eslint parser?

kylegach · 2022-10-12T15:56:40Z

code/lib/cli/rendererAssets/react/ts-legacy/Button.stories.tsx

+import type { Meta, StoryObj } from '@storybook/react';
+import { Button } from './Button';
+
+// More on default export: https://storybook.js.org/docs/react/writing-stories/introduction#default-export


This comment also applies to the legacy files.

Yes, I separated the default export and meta as well for the "legacy" templates, for 2 reasons:

It is more typesafe than as, as as will swallow some errors for you.

It already moves people closer to the new format, so it is easier to migrate when they are ready to adopt TS 4.9.

kasperpeulen · 2022-10-12T18:54:01Z

@kylegach Thanks for the review! This PR is a bit of a mess, as I stopped ignoring the templates (except for aurelia) and test files in tsc, and fixed all the tsc errors, and bumped TS to 4.9. But glad you could find the important files 😅

kasperpeulen · 2022-10-17T09:06:38Z

code/lib/cli/rendererAssets/react/ts-legacy/Button.stories.tsx

+
+export default meta;
+
+export const Primary: StoryObj<typeof Button> = {


type Story = StoryObj<typeof Button>;

socket-security · 2022-10-18T12:45:56Z

Socket Security Report

Dependency issues detected. If you merge this pull request, you will not be alerted to the instances of these issues again.

📜 New install scripts detected

A dependency change in this PR is introducing new install scripts to your install step.

Package	Script field	Location
@ampproject/[email protected] (added)	`postinstall`	`code/examples/external-docs/package.json` via [email protected], [email protected], [email protected]

Socket.dev scan summary

Issue	Status
Did you mean?	✅ no new possible package typos
Install scripts	⚠️ 1 new install script detected
Telemetry	✅ no new telemetry
Troll package	✅ no new troll packages
Malware	✅ no new malware
Native code	✅ no new native modules

Bot Commands

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore [email protected] [email protected]

@SocketSecurity ignore @ampproject/[email protected]

⚠️ Please accept the latest app permissions to ensure bot commands work properly. Accept the new permissions here.

Powered by socket.dev

shilman

@kasperpeulen Please split the TS upgrade into its own PR. The CLI template changes can telescope off of it.

ndelangen · 2022-10-23T15:49:59Z

code/lib/cli/src/automigrate/fixes/cra5.test.ts

@@ -1,12 +1,19 @@
 /* eslint-disable no-underscore-dangle */
+import { StorybookConfig } from '@storybook/core-common/src';


this will be broken when published to npm, because the src directory won't get published

kasperpeulen · 2022-10-28T10:26:14Z

@kylegach @jonniebigodes
I split this PR in 3, I guess for you guys this one is interesting:
#19665

CSF3 react templates for storybook init

613171e

kasperpeulen added feature request typescript csf3 labels Oct 12, 2022

kasperpeulen commented Oct 12, 2022

View reviewed changes

.vscode/settings.json Outdated Show resolved Hide resolved

kasperpeulen commented Oct 12, 2022

View reviewed changes

kylegach reviewed Oct 12, 2022

View reviewed changes

address review

438f04e

kasperpeulen commented Oct 17, 2022

View reviewed changes

yannbf marked this pull request as draft October 17, 2022 14:21

yannbf changed the title ~~WIP: CSF3 react templates for storybook init~~ CSF3 react templates for storybook init Oct 17, 2022

Merge branch 'next' into future/new-react-ts-templates

e2c1800

ndelangen assigned kasperpeulen Oct 18, 2022

shilman requested changes Oct 18, 2022

View reviewed changes

kasperpeulen mentioned this pull request Oct 18, 2022

Svelte: Improve CSF3 types #19512

Merged

Merge branch 'next' into future/new-react-ts-templates

d42ea8c

ndelangen reviewed Oct 23, 2022

View reviewed changes

kasperpeulen mentioned this pull request Oct 27, 2022

Upgrade to TS 4.9 #19646

Closed

kasperpeulen closed this Oct 28, 2022

ndelangen deleted the future/new-react-ts-templates branch November 28, 2022 09:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CSF3 react templates for storybook init #19462

CSF3 react templates for storybook init #19462

kasperpeulen commented Oct 12, 2022 •

edited

Loading

kasperpeulen Oct 12, 2022

ndelangen Oct 23, 2022 •

edited

Loading

kylegach Oct 12, 2022

kasperpeulen Oct 12, 2022

jonniebigodes Oct 12, 2022

kasperpeulen Oct 13, 2022

kylegach Oct 12, 2022

kasperpeulen Oct 12, 2022

kasperpeulen Oct 12, 2022

ndelangen Oct 18, 2022

kylegach Oct 12, 2022

kasperpeulen Oct 12, 2022

kasperpeulen commented Oct 12, 2022

kasperpeulen Oct 17, 2022 •

edited

Loading

socket-security bot commented Oct 18, 2022

shilman left a comment •

edited

Loading

ndelangen Oct 23, 2022

kasperpeulen commented Oct 28, 2022


		export default meta;

		export const Primary: StoryObj<typeof Button> = {

		@@ -1,12 +1,19 @@
		/* eslint-disable no-underscore-dangle */
		import { StorybookConfig } from '@storybook/core-common/src';

CSF3 react templates for storybook init #19462

CSF3 react templates for storybook init #19462

Conversation

kasperpeulen commented Oct 12, 2022 • edited Loading

What I did

Blocked by

How to test

Choose a reason for hiding this comment

ndelangen Oct 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kasperpeulen commented Oct 12, 2022

kasperpeulen Oct 17, 2022 • edited Loading

Choose a reason for hiding this comment

socket-security bot commented Oct 18, 2022

shilman left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kasperpeulen commented Oct 28, 2022

kasperpeulen commented Oct 12, 2022 •

edited

Loading

ndelangen Oct 23, 2022 •

edited

Loading

kasperpeulen Oct 17, 2022 •

edited

Loading

shilman left a comment •

edited

Loading