feat: storybook-builder and storybook-framework-web-components #2437

bashmish · 2023-08-31T17:02:02Z

Fixes #2025

This is mostly a copy of

https://github.com/bashmish/storybook-builder-wds
https://github.com/bashmish/storybook-web-components-wds
with renaming, docs restructuring and a few small fixes

Questions

mark as beta state? in docs or/and in package version
- release with current set of features, that's OK
what to do with old plugin docs and guide? mark as deprecated and link to Migration guide?
- keep old plugin and docs
- for now do not link anywhere or deprecate anything
- share the builder with more developers to test it
- once we have more feedback and add more features (e.g. MDX and such), we can deprecated the old WDS plugin and phase it out gradually
make a Guide section now? do we need a guide or docs is sufficient?
- yes, will be a separate PR

TODOs

fix npm run update:mjs-dts-entrypoints
fix types issue in the build

changeset-bot · 2023-08-31T17:02:06Z

🦋 Changeset detected

Latest commit: 659ed7e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@web/storybook-builder	Minor
@web/storybook-framework-web-components	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Westbrook · 2023-08-31T19:18:58Z

I love it! Especially all the docs you've already included 👨🏼‍🎤

Things I know we need before something like this could land:

I know there are naming conventions in the SB ecosystem, but shoudl this be @web/dev-server-storybook@2.0?
a working version of @web/dev-server-storybook@1.x, 😞, regardless of which name we use. Skipping 1.x all together could be an option, but I'm not willing to make that decision for myself, just yet. Hopefully @koddsson can make some progress on that, soon 🤞🏼
if we don't use @web/dev-server-storybook
- WDS Storybook needs to be deprecated officially. That means no code/docs/etc. remain.
- should the new packages jump to 1.0?
CI needs to pass
- right now it's red, but likely we need more things, too
- we need to run Storybook in CI
- we need to build Storybook in CI
- should it just be kitchen sink storybook, or do we need to do various add-on combinations?
- should Storybook do anything else in CI?
is it possible to not ship a CJS configuration?

Other than the code mostly working for me, I've not looked to closely at it, but once we start getting answers to the above, I am excited to do so!

LarsDenBakker · 2023-08-31T21:05:34Z

packages/storybook-builder/src/generate-stories-script.ts

+      logger.warn(`Cannot process ${ext} file with storyStoreV7: ${relativePath}`);
+    }
+
+    return `  '${toImportPath(relativePath)}': () => import('${process.cwd()}/${relativePath}')`;


Might need to take care of windows file paths in dynamic imports here?

thanks, I'll normalize the right path too

I'm not sure it's actually needed, I checked what happens then in packages/dev-server-rollup/src/rollupAdapter.ts and seems like the path will be normalized before doing the check which resolves this path correctly, but there are a few checks that are done on the non-normalized one before it which might go wrong

I also doubt now that the left path is normalized good for Windows

I wish I could debug on Windows myself and see if there is a problem and where, but on M1 I can't, so until that I'd rather keep things as is, or I'll have to guess both the problem and solution :D

the long-term plan is to have automated tests and run a Storybook in the build, so I think we can also run one in our Windows pipeline

shilman · 2023-09-01T14:35:04Z

This looks amazing -- incredible job @bashmish !!! Is there an easy way for me to test this out?

Westbrook · 2023-09-01T14:37:50Z

@shilman we're testing the dev release in SWC right now (via the releases made against @bashmish's own repo of this code)

Only one weirdness in Storybook to make it work.

shilman · 2023-09-01T14:39:53Z

lol i wrote that "weirdness". Apologies! I'll merge and release a patch to fix if that helps things along.

bashmish · 2023-09-01T15:35:06Z

I love it! Especially all the docs you've already included 👨🏼‍🎤

@Westbrook Thanks for the compliment!

Things I know we need before something like this could land:

I know there are naming conventions in the SB ecosystem, but shoudl this be @web/dev-server-storybook@2.0?

@web/dev-server-storybook is a plugin for WDS which connects Storybook to it
@web/storybook-builder is a plugin for Storybook which connects WDS to it
so the relationship between WDS and Storybook has turned around with the Builders architecture

our own convention for WDS plugins is @web/dev-server-plugin-name, but @web/storybook-builder is not a plugin for WDS, so I thought it shouldn't be @web/dev-server-storybook or start with @web/dev-server-

For the same reason I made a new top section in the documentation, instead of fitting it into the WDS as a subsection.

Storybook package naming convention were hard to follow too after moving inside modern-web:
I could just add @web prefix and make
@web/storybook-builder-wds
@web/storybook-web-components-wds
but

in other packages we don't add wds, e.g. instead of @web/test-runner-wds we just use @web/test-runner
- so I renamed @web/storybook-builder-wds to @web/storybook-builder
- and first renamed @web/storybook-web-components-wds to @web/storybook-web-components
in other plugins we usually put the grouping name in the front, e.g. @web/dev-server-plugin-name
- so I decided to have a clear grouping name for frameworks (like rollup-pugin-) so that they are all ordered next to each other and renamed further @web/storybook-web-components to @web/storybook-framework-web-components

a working version of @web/dev-server-storybook@1.x, 😞, regardless of which name we use. Skipping 1.x all together could be an option, but I'm not willing to make that decision for myself, just yet. Hopefully @koddsson can make some progress on that, soon 🤞🏼

I think @koddsson is busy fixing it

if we don't use @web/dev-server-storybook

WDS Storybook needs to be deprecated officially. That means no code/docs/etc. remain.

This is also one of my questions in the PR description
I agree we should deprecate it, but not sure we should remove docs right away. Maybe keep it in place with links to Migration for the time being?

should the new packages jump to 1.0?

I think too early, because some of the features are yet to be implemented, mainly:

CLI for init
CLI for migration
MDX
MDJS
HMR

CI needs to pass

right now it's red, but likely we need more things, too

Yeah, some TS errors which I didn't have locally. I'll make sure to fix them.

we need to run Storybook in CI

we need to build Storybook in CI

should it just be kitchen sink storybook, or do we need to do various add-on combinations?

should Storybook do anything else in CI?

Fully agree on that. Recent issues with 1.x show the urgency for it.
We can learn from Storybook monorepo how to test it, I saw they have generators for various combinations, which is a must for them to test multiple builders and frameworks. Though we have only one now, so can simplify it for the time being.

is it possible to not ship a CJS configuration?

This week I tested the ESM version and couldn't make it work, but I didn't investigate why yet.
The only thing I saw is that Storybook expected a CJS module of a framework.
IIRC, Storybook has plans to make all dependencies loaded in the browser to be ESM in Storybook 8 which is a lot of work, but on the Node level I don't see why we can't make it work already now, especially given .storybook/ config files support ESM now.
This is not the highest prio for me right now.

Other than the code mostly working for me, I've not looked to closely at it, but once we start getting answers to the above, I am excited to do so!

Great! I'm open to discuss anything to make sure this is reviewable and goes in the right direction.

shilman · 2023-09-02T00:13:36Z

FWIW weirdness fixed here https://github.com/storybookjs/storybook/releases/tag/v7.5.0-alpha.0

bashmish · 2023-09-04T19:22:43Z

@Westbrook you can also remove https://github.com/adobe/spectrum-web-components/blob/storybook/patches/%40chialab%2Bestransform%2B0.17.4.patch and update to @chialab/estransform@0.17.5 as it was fixed in there:

docs/docs/storybook-builder/configuration.md

docs/docs/storybook-builder/migration-to-storybook-7.md

docs/docs/storybook-builder/overview.md

packages/storybook-builder/src/rollup-plugin-prebundle-modules.ts

koddsson · 2023-09-11T09:28:30Z

a working version of @web/dev-server-storybook@1.x, 😞, regardless of which name we use. Skipping 1.x all together could be an option, but I'm not willing to make that decision for myself, just yet. Hopefully @koddsson can make some progress on that, soon 🤞🏼

I think @koddsson is busy fixing it

It should all be fixed now 🤞🏻

thepassle · 2023-09-11T11:02:24Z

lgtm :)

Westbrook mentioned this pull request Aug 31, 2023

[dev-server-storybook] v1 published files are wrong format #2405

Closed

LarsDenBakker reviewed Aug 31, 2023

View reviewed changes

shilman mentioned this pull request Sep 1, 2023

Docs: Fix TOC import storybookjs/storybook#24047

Merged

8 tasks

bashmish force-pushed the feat/storybook-builder branch 7 times, most recently from 272248e to b69116b Compare September 4, 2023 14:42

thepassle reviewed Sep 11, 2023

View reviewed changes

bashmish force-pushed the feat/storybook-builder branch from f313fea to 9963d96 Compare September 11, 2023 10:32

thepassle approved these changes Sep 11, 2023

View reviewed changes

bashmish force-pushed the feat/storybook-builder branch from 9963d96 to d412c84 Compare September 11, 2023 11:44

feat: storybook-builder and storybook-framework-web-components

659ed7e

bashmish force-pushed the feat/storybook-builder branch from d412c84 to 659ed7e Compare September 11, 2023 11:50

bashmish merged commit 17ab474 into master Sep 11, 2023

bashmish deleted the feat/storybook-builder branch September 11, 2023 12:00

bashmish mentioned this pull request Mar 2, 2024

chore: clean up dev dependencies #2659

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: storybook-builder and storybook-framework-web-components #2437

feat: storybook-builder and storybook-framework-web-components #2437

bashmish commented Aug 31, 2023 •

edited

Loading

changeset-bot bot commented Aug 31, 2023 •

edited

Loading

Westbrook commented Aug 31, 2023

LarsDenBakker Aug 31, 2023

bashmish Sep 1, 2023

bashmish Sep 4, 2023

shilman commented Sep 1, 2023

Westbrook commented Sep 1, 2023 •

edited

Loading

shilman commented Sep 1, 2023

bashmish commented Sep 1, 2023

shilman commented Sep 2, 2023

bashmish commented Sep 4, 2023

koddsson commented Sep 11, 2023

thepassle commented Sep 11, 2023

feat: storybook-builder and storybook-framework-web-components #2437

feat: storybook-builder and storybook-framework-web-components #2437

Conversation

bashmish commented Aug 31, 2023 • edited Loading

Questions

TODOs

changeset-bot bot commented Aug 31, 2023 • edited Loading

🦋 Changeset detected

Westbrook commented Aug 31, 2023

LarsDenBakker Aug 31, 2023

Choose a reason for hiding this comment

bashmish Sep 1, 2023

Choose a reason for hiding this comment

bashmish Sep 4, 2023

Choose a reason for hiding this comment

shilman commented Sep 1, 2023

Westbrook commented Sep 1, 2023 • edited Loading

shilman commented Sep 1, 2023

bashmish commented Sep 1, 2023

shilman commented Sep 2, 2023

bashmish commented Sep 4, 2023

koddsson commented Sep 11, 2023

thepassle commented Sep 11, 2023

bashmish commented Aug 31, 2023 •

edited

Loading

changeset-bot bot commented Aug 31, 2023 •

edited

Loading

Westbrook commented Sep 1, 2023 •

edited

Loading