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

chore(storybook): update v1 syntax to use Storybook v6 #568

Draft
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

agilgur5
Copy link
Member

@agilgur5 agilgur5 commented Jul 18, 2024

Follow-up to #432 (comment) and make the v1 code Storybook v7 compatible. This is just one step toward the upgrade, but perhaps the most breaking one.
Follow-up to #567 "Future Work" part 1.

Motivation

  • while the deps used Storybook v6, the configuration and stories themselves were still on a legacy v5 format
    • v6 had backward compat for v5, but v7 does not, so this needs upgrading/migrating
      • or well, v7 has some legacy mode for it that can be enabled with some config, but it is entirely gone in v8

Modifications

Verification

yarn start works the same as it did in #567

Notes to Reviewers

This requires #567 to work and so is built on top of it. Please do not merge until that is merged first!

- go back to Storybook v6 addons, matching the `storybook` dep
  - the addons were partially auto-upgraded to breaking versions by dependabot
- go back to Webpack v4, matching Storybook v6
  - this was also auto-upgraded to a breaking version by dependabot
  - go back to `ts-loader` v8, matching Webpack v4
    - and this too had a breaking dependabot upgrade

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
- the latter took some manual editing and resolutions until it got it, and now it reproduces the minimal variant

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
- as per its archived docs
- tried to make the diff as small as possible

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
@agilgur5 agilgur5 force-pushed the chore-update-v1-storybook-v6-syntax branch from a28fd4d to 8a3ec46 Compare July 19, 2024 16:10
- same as downstream in Workflows, in order to code split properly, can't use the `src/` or `src/components/` imports as they have side effects (and therefore can't be tree-shaken)
  - instead have to do individual component imports to tree shake properly

- this seems to substantially improve performance from lethargic minutes of loading on first page load to seconds
  - I believe Storybook tries to code split things itself, so maybe it was making a giant bundle for each story file?

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
- while the deps used Storybook v6, the configuration and stories themselves were still on a legacy v5 format
  - v6 had backward compat for v5, but v7 does not, so this needs upgrading/migrating
    - or well, v7 has some legacy mode for it that can be enabled with some config, but it is entirely gone in v8
  - migrate `storiesOf` to CSF per https://storybook.js.org/docs/7/migration-guide#storiesof-to-csf
    - then had to do a bunch of manual changes to get back mostly the same previous indentation (with some minor differences where there were mistakes or inconsistencies)
      - tried to keep the diff as small as possible
    - CSF is still valid through to latest v8

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
- modified version of v2's config

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
- `ts-loader` is necessary to handle the `import type` syntax
  - custom `tsconfig.json` doesn't seem necessary though
- use same SASS config
- plain CSS config not needed though bc PostCSS already runs by default

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
- the conventional format
  - also had some problems finding stories without this change

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
- per https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#hoisted-csf-annotations
  - migrate `.story.name` -> `.storyName`
    - not sure why the automigration didn't do this and instead used the deprecated name 😕

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
- remove `ts-loader` as not necessary
- plus some settings for Storybook's TS loader preset

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
@agilgur5 agilgur5 force-pushed the chore-update-v1-storybook-v6-syntax branch from 8a3ec46 to ea006e0 Compare July 19, 2024 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant