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: Does not install lit-html in new web components project #15851

Closed
shilman opened this issue Aug 16, 2021 · 6 comments
Closed

CLI: Does not install lit-html in new web components project #15851

shilman opened this issue Aug 16, 2021 · 6 comments

Comments

@shilman
Copy link
Member

shilman commented Aug 16, 2021

sb init doesn't install lit-html automatically, and it should. See attached conversation:

Because @storybook/web-components requires lit-html as a peer dependency. If you install with sb init which is the recommended way to install I believe it adds that automatically.

Originally posted by @shilman in #15835 (reply in thread)

@ben-m-j-taylor
Copy link

Hi @shilman, I'd like to pick this issue up if possible.

@ben-m-j-taylor
Copy link

Currently the cli doesn't add peerDependencies for any of the supported frameworks/templates so theoretically this issue could arise for any supported framework, although this is pretty unlikely that this would ever cause an issue if storybook is being added to an existing project but i suppose it could be an issue if storybook were being added at the same time a project is created (maybe I'm clutching at straws a bit there).

It would probably be worth adding handling for peerDependencies for all frameworks/templates, there is one caveat to this though as not all versions of npm will install missing peerDependencies and yarn doesn't install them at all.

npm versions 1, 2, and 7 will automatically install peerDependencies if they are not explicitly depended upon higher in the dependency tree. For npm versions 3 through 6, you will receive a warning that the peerDependency is not installed instead.
Ref: https://nodejs.org/en/blog/npm/peer-dependencies/

So the best solution would probably be to have some handling that maps the peerDependencies required for a given framework/template into devDependencies when we generate the package.json, assuming this wouldn't cause any undesirable side effects.

Do you have any thoughts on this approach @shilman?

@shilman
Copy link
Member Author

shilman commented Sep 11, 2021

@ben-m-j-taylor Thanks for taking this on! 🙏

I don't think it needs to take any of the frameworks on as peer dependencies since it doesn't actually USE any of them. It installs packages into the user's project, but that's not the same thing as relying on any of the functionality/code from the package. cc @gaetanmaisse

@ben-m-j-taylor
Copy link

Hey @shilman, sorry for the slow reply!

Yes, you're right, I think my mental model of how this was all working had gotten a bit spun around.

So really what we want to do is take what storybook is listing as a peer dependency for a given template/framework and if it isn't already installed in the project we should add it as a dependency and install it.

So for @storybook/web-components which has the following peer dependencies in its package.json:

{
  "name": "@storybook/web-components",
  "version": "6.4.0-alpha.32",
  ...
  "peerDependencies": {
    "lit-html": "^1.4.1 || ^2.0.0-rc.3"
  },
  ...
}

We would need to ensure that lit-html is a dependency of the initialised project if they were to run npx sb init --type web_components.

@RabbitEarsDesign
Copy link

Is this still outstanding? I am interested in tackling if we still need to complete it

@shilman
Copy link
Member Author

shilman commented Jan 3, 2022

Great Caesar's ghost!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.5.0-alpha.6 containing PR #17106 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

Closing this issue. Please re-open if you think there's still more to do.

@shilman shilman closed this as completed Jan 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants