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

cli: web-components stories homogenization #10703

Merged
merged 3 commits into from
May 13, 2020

Conversation

tooppaaa
Copy link
Contributor

@tooppaaa tooppaaa commented May 9, 2020

Issue:

We need similar stories after generation to run e2e against

Part of #10702

@tooppaaa tooppaaa force-pushed the generator_webComponentsStories branch from adaed81 to 271c230 Compare May 9, 2020 18:47
@tooppaaa tooppaaa changed the title cli: react-scripts stories homogenization cli: web-components stories homogenization May 9, 2020
Comment on lines +40 to +44
<p class="note">
<b>NOTE:</b> <br />
Have a look at the <span class="inline-code">.storybook/webpack.config.js</span> to add webpack
loaders and plugins you are using in this project.
</p>
Copy link
Member

Choose a reason for hiding this comment

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

I think we can remove that as I'm not sure there will be a .storybook/webpack.config.js

Copy link
Member

Choose a reason for hiding this comment

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

You are right, the template does not contain such file:
image

However it's nice to tell people that if they add a webpack.config.js file it will be picked up, so maybe just rephrase that? Also, that sentence is in every framework's template, so if we were to make a change, it has to be synced with the others.

@shilman what's your take on this?

Copy link
Member

Choose a reason for hiding this comment

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

AFAIK the current recommendation is to override webpackFinal in main.js, so we should probably update all the docs accordingly. webpack.config.js is still supported (for now, not sure how long we plan to keep this? But we should be directing everybody towards the recommendation. @ndelangen please feel free to correct if I got any of this wrong.

Copy link
Member

Choose a reason for hiding this comment

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

the current recommendation is to override webpackFinal in main.js

indeed

lib/cli/src/generators/WEB-COMPONENTS/index.js Outdated Show resolved Hide resolved
`;

Emoji.story = {
parameters: { notes: 'My notes on a button with emojis' },
Copy link
Member

Choose a reason for hiding this comment

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

I think we should get rid of notes for every framework. It's deprecated or was even removed already.
@ndelangen what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

You're right @yannbf, notes has been moved to the repo storybookjs/deprecated-addons so no 6.0 version will be released.

Copy link
Member

@shilman shilman May 11, 2020

Choose a reason for hiding this comment

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

Agree we should get rid of notes and pick a better example. Maybe backgrounds and/or docs?

Slight clarification on the deprecation. We won't release a 6.0 version of notes along with the Storybook release. We will release deprecated addons with the community on an independent schedule and will be looking for users of those addons to step up as maintainers if they want to keep them going

Copy link
Member

Choose a reason for hiding this comment

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

I agree, we should leave it out for now.

@yannbf yannbf mentioned this pull request May 10, 2020
2 tasks
Copy link
Member

@yannbf yannbf left a comment

Choose a reason for hiding this comment

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

@tooppaaa the other points have been resolved and will be done in a different PR, but can you check the one about removing @storybook/addons? then we're good to merge!

…g `sb init` in a web-component project

`@storybook/addons` is already a dependency of all `@storybook/addon-XXX` packages so there is no need to add it to the root project directly.
@gaetanmaisse
Copy link
Member

Tested ✅ @storybook/addons is already a dependency of all @storybook/addon-XXX packages so there is no need to add it to the root project directly.

@ndelangen ndelangen merged commit c0dae38 into next May 13, 2020
@ndelangen ndelangen deleted the generator_webComponentsStories branch May 13, 2020 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli maintenance User-facing maintenance tasks web-components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants