open-wc / open-wc

Open Web Components: guides, tools and libraries for developing web components.
https://open-wc.org/
MIT License
2.25k stars 423 forks source link

Using Multiple Lit Versions Breaks Storybook #2418

Open evanjo03 opened 2 years ago

evanjo03 commented 2 years ago

I am developing an application, and am stuck on an issue where storybook doesn't seem to be able to process my html templates. I receive the following error in the browser when I try and run SB:

Did you forget to return the HTML snippet from the story?
Use "() => <your snippet or node>" or when defining the story.

I am currently upgrading my application to lit from lit-html/element, so while my packages are using lit, they have dependencies that are still running on lit-html/element.

I saw https://github.com/open-wc/open-wc/issues/2322 and am wondering if the issue may be related, or something where storybook is getting tripped up when there are multiple versions of lit deps in the tree. If I remove the legacy dependencies using lit-element and lit-html, it works fine.

I have created a sample repository here with the most basic setup that can cause this issue; here's everything that I did:

  1. Create application with @open-wc initializer (with default values for things, include storybook, lit@2.x.x, etc).
  2. Create web component with @open-wc initializer, but use legacy lit packages (lit-element@2.x.x, lit-html@1.x.x).
  3. Publish the legacy web component from step 2 and add it as a dependency to the application from step 1.
  4. Run storybook

There are some workarounds out there, like the one mentioned in this issue. Additionally, including the latest versions of lit-element and lit-html as devDependencies at the root level of the monorepo seems to work. That said, these are both less-than-ideal long-term.

ChristianUlbrich commented 2 years ago

Storybook is a mess. Basically the problem is, that they are relying on a instanceof check in their code base against lit-html, to automagically support Lit templates. This is a very bad design idea and tightly couples @storybook/webcomponents to a particular peer dependency of lit-html.

I have not dived into the delicate details of lit-html vs. lit, why it actually works to render a Story with lit and having it being an instanceof lit-html.TemplateResult... For me it works™ , if you have lit-html installed, as well as the latest lit package, while using lit exclusively in our stories.

The problematic code is in render.ts.

The zero config approach of @storybook/webcomponents of secretively supporting lit templates should not relay on an instanceof check, but on a duck typing approach (i.e. has the provided TemplateResult the roughly correct shape. However there is nothing, that @open-wc could do about, this is entirely Storybook's fault.

Hopefully I find the time for a proper PR for Storybook; fix should be trivial...

Westbrook commented 2 years ago

@ChristianUlbrich the latest releases do not rely on instanceof https://github.com/storybookjs/storybook/blob/next/app/web-components/src/client/preview/render.ts#L21 That version is currently applied in web/dev-server-storybook, which is the recommended approach at this time when leveraging out tools.

@evanjo03 the long term solution is to not attempt to support lit@1 and lit@2 at the same time. The work arounds listed are “work arounds” because that is not an optimal long term solution for a project. Due to Storybook and OpenWC not knowing which version of Lit is the “host” of your project there’s little more that can be done in the currently available code base. Storybook has mentioned time and again that they are interested in renderer injection to support a future where not only could you have multiple versions of lit but you could have angular, Preact, and lit components all side by side, but this seams like a far future goal.

robrez commented 2 years ago

I have observed problems while trying to migrate from a "lerna" monorepo to "npm workspaces". Created several branches over here to test out a few different setups:

(edit: check indicates that it "works as expected")

It seems that npm workspaces in particular are problematic. Speculatively something about module resolution there that differs from the other variations

Westbrook commented 2 years ago

@robrez can you link to the NPM workspaces monorepo context so I can take a look. Usually it’s just a question of ensuring that the “default” lit-html is 2.0 which involves directly depending on it when other packages depend on lit-html@1.0 but I don’t work with NPM to know 100% that that works there. When you’ve got the non-working context set up, I’ll take a look.

robrez commented 2 years ago

Here is the non working branch - https://github.com/robrez/openwc-testing/tree/npm-workspaces-storybook

All of the branches are linked from README.md on the main branch.

btopro commented 2 years ago

I found a work around for this that's a bit less aggressive. I have a postinstall hook that runs a bash script that does the following:

cd node_modules/storybook-prebuilt
yarn install --prod

I then use resolutions to force a single copy of lit, lit-html and lit-element into node_modules/ so that all other code in the repo can use that version of lit, yet when the storybook goes to get built it'll have a localized lit-html as its the only dependency there. Hope this helps.

smurugavels commented 2 years ago

Thanks @robrez for pointing me here. I was able to fix the versioning problem from above comments.

Additionally, I had to upgrade these libs to current available versions to make my projects work. Hopefully, this saves someone's time.

Lit version : "lit": "^2.2.7",

other libs: "@open-wc/scoped-elements": "^2.1.2", "lit-html": "^2.2.6", "@open-wc/testing": "^3.1.6", "@web/dev-server-storybook": "^0.5.1"

Additional info: I am using npm workspaces I removed package.lock.json that was holding on to bad references, and did a fresh install to make sure lit 2 upgrade works