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

Support for lit-element with storybook/polymer #4958

Closed
tonai opened this issue Dec 10, 2018 · 38 comments · Fixed by #8400
Closed

Support for lit-element with storybook/polymer #4958

tonai opened this issue Dec 10, 2018 · 38 comments · Fixed by #8400

Comments

@tonai
Copy link
Contributor

tonai commented Dec 10, 2018

Is your feature request related to a problem? Please describe.
lit-element seems to be the next evolution of polymer, but using the @storybook/polymer package with lit-element is not working out of the box.

Describe the solution you'd like
There is a little configuration that must be added in the webpack config file in order to make the storybook work with lit-element.

Describe alternatives you've considered
You may override the webpack configuration yourself (in the .storybook/webpack.config.js file) with:

module.exports = (baseConfig, env, config) => {
  config.module.rules[0].exclude = /node_modules\/(?!(lit-html|@polymer)\/).*/;
  return config;
};

Are you able to assist bring the feature to reality?
The solution is to add the previous exclude line directly in the webpack configuration.

Additional context
The exclude regexp may contain more elements like /node_modules\/(?!(@webcomponents\/shadycss|lit-html|@polymer|@vaadin|@lit)\/).*/ (discussion here: lit/lit-element#54 (comment)).

@web-padawan
Copy link
Contributor

There are reasons to make a separate config:

  1. polymer-webpack-loader is not needed for Polymer 3 and LitElement
  2. @webcomponents/webcomponentsjs polyfill should be updated to 2.x, not 1.x

The only breaking change in the polyfill is that 1.x includes HTML imports polyfill, which is no longer needed by Polymer 3 and LitElement.

Also, version 2.x of the polyfill brings own Symbol polyfill instead, but it should not conflict with those from core-js. It would make sense to wait for webcomponents/webcomponentsjs#1021 though.

@stale
Copy link

stale bot commented Jan 19, 2019

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Jan 19, 2019
@web-padawan
Copy link
Contributor

I will work on this issue, please don't close 😁

@stale stale bot removed the inactive label Jan 19, 2019
@daKmoR
Copy link
Contributor

daKmoR commented Jan 19, 2019

yes it would probably nice to have a separate distribution... something that could be smaller and does not need the polymer-webpack-loader...

maybe call it @storybook/lit or @storybook/lit-html as it will only need lit-html and not lit-element to "work"...

I think a whitelist on what to compile will always be in the realm of the users themselves => but there should definitely be a good docu about it.

@web-padawan
Copy link
Contributor

@storybook/web-components makes even more sense to me, actually.
One thing we really need is "bare" polyfills though, will ping Polymer team again about that.

@web-padawan
Copy link
Contributor

web-padawan commented Feb 5, 2019

Now when LitElement stable version is released, it makes sense to work on that so we could get this into the 5.0.0 storybook release.

@ndelangen I'm going to work on this and start by adapting the code from @storybook/polymer, especially the following steps:

  1. add config to transpile lit-element and lit-html dependencies
  2. get rid of polymer-webpack-loader
  3. get rid of HTML imports polyfill, use individual polyfills

Could you briefly describe what parts do I need to cover, apart from these changes?

Also, note that possible alternative would be to drop Polymer 2 and incorporate LitElement into Polymer package, and make sure that only Polymer 3 + LitElement are supported.

This makes sense because of the fact that Polymer 3 and LitElement use the same versions of web components polyfills, the story setup can be shared, etc.

@jackblackCH
Copy link

jackblackCH commented Feb 5, 2019

+1, we are creating a new storybook with lit-element right now and are welcome to contribute.

@jackblackCH
Copy link

jackblackCH commented Feb 5, 2019

I suggest:

  1. Create a separate @storybook/webcomponents package
  2. Should support ES6 classes and can handle the polyfills (rel [Polymer] Bundling ES5 shim breaks ES6 elements #3497)

Example: button.js component

import {html, LitElement} from 'lit-element';

class CustomButton extends LitElement {
    render() {
      return html`
        <button>🚀Rocket Science<slot></slot></button>
      `;
    }
}

customElements.define('custom-button', CustomButton);

Story:

import { document, console } from 'global';
import { storiesOf } from '@storybook/webcomponents';
import 'custom-button.js';

storiesOf('Buttons', module)
  .add('custom-button', () => {
    const el = document.createElement('custom-button');
    return el;
  })
  .add('custom-button-alternative', () => {
    return html`<custom-button></custom-button>`;
  });

@shilman
Copy link
Member

shilman commented Feb 9, 2019

Hurrah!! I just released https://github.com/storybooks/storybook/releases/tag/v5.0.0-beta.2 containing PR #5490 that references this issue. Upgrade today to try it out!

Because it's a pre-release you can find it on the @next NPM tag.

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

@shilman shilman closed this as completed Feb 9, 2019
@web-padawan
Copy link
Contributor

@shilman that PR was only one prerequisite.
Please see #4958 (comment) for a full list of the actions to implement.

I can try to refactor @storybook/polymer to make this happen, with dropping Polymer 2.
So could you reopen this issue please?

@Keraito
Copy link
Contributor

Keraito commented Feb 9, 2019

Sounds logical @web-padawan, reopening this issue for you guys.

@Keraito Keraito reopened this Feb 9, 2019
@web-padawan
Copy link
Contributor

web-padawan commented Feb 9, 2019

@Keraito thanks for reopening. Could you also help me with clarifying some details about how can we tweak webpack setup used by Storybook?

I have created a very basic example of the webpack + web components here:
https://github.com/web-padawan/webpack-wc-demo

So the steps I would like to configure are described below:

  1. replace babel-loader with the loader provided by webpack-babel-multi-target-plugin
  module: {
    rules: [
      {
        test: /\.js$/,
        use: [
          BabelMultiTargetPlugin.loader()
        ]
      }
    ]
  },
  1. Add the BabelMultiTargerPlugin itself:
    new BabelMultiTargetPlugin({
      babel: {
        // @babel/preset-env options common for all bundles
        presetOptions: {
          useBuiltIns: 'usage'
        }
      },

      // Target browsers with and without ES modules support
      targets: {
        es6: {
          browsers: [
            'last 2 Chrome major versions',
            'last 2 ChromeAndroid major versions',
            'last 2 Edge major versions',
            'last 2 Firefox major versions',
            'last 3 Safari major versions',
            'last 3 iOS major versions'
          ],
          tagAssetsWithKey: false, // don’t append a suffix to the file name
          esModule: true // marks the bundle used with <script type="module">
        },
        es5: {
          browsers: ['ie 11'],
          tagAssetsWithKey: true, // append a suffix to the file name
          noModule: true // marks the bundle included without `type="module"`
        }
      }
    })

That's basically all that we need to get ES module build for evergreen browsers + ES5 build for "nomodule" browsers, especially IE11 which is still around. Also this would fix #3497

Is this feasible to achieve by tweaking framework preset only? Or the changes of this kind should be considered as part of Storybook core? I would like to hear your feedback about this feature.

UPD: submitted #5531 as this makes sense to discuss as a separate feature request.

@Keraito
Copy link
Contributor

Keraito commented Feb 9, 2019

Unfortunately I'm not the best person to ask webpack related issues, tagging @igor-borisov for you, who should have more knowledge in this domain

@ndelangen
Copy link
Member

Is this feasible to achieve by tweaking framework preset only? Or the changes of this kind should be considered as part of Storybook core? I would like to hear your feedback about this feature.

@igor-dv 👆 Do you know?

@ndelangen
Copy link
Member

@web-padawan looking at what that plugin does, I think that's something we want in the core itself, so all frameworks can benefit from choosing to either run in a modern environment or a legacy one.

Can we schedule a online meeting to discuss perhaps?

@web-padawan
Copy link
Contributor

@ndelangen thanks for taking a look! Let's continue discussion in #5531
If you think online meeting would be more productive, we could arrange it.

@ndelangen
Copy link
Member

@web-padawan Please join our discord if you can, I'm super active there.
https://discord.gg/sMFvFsG

When would you be available?

@stale
Copy link

stale bot commented Mar 5, 2019

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot closed this as completed May 1, 2019
@shilman shilman reopened this May 2, 2019
@stale stale bot removed the inactive label May 2, 2019
@shilman
Copy link
Member

shilman commented May 2, 2019

@web-padawan any movement on this? seems like there are a lot of people lined up to use it!

@stale
Copy link

stale bot commented May 23, 2019

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label May 23, 2019
@web-padawan
Copy link
Contributor

I'm still interested in this issue and hopefully will have time to finally work on it soon.
Please do not close it in the meanwhile.

@stale stale bot removed the inactive label May 23, 2019
@jokin
Copy link

jokin commented May 23, 2019

Have you looked at the open-wc tool? they have a feature to use storybooks with lit-element
at https://open-wc.org/demoing/

@web-padawan
Copy link
Contributor

Sure, I actually use that. It's based on @storybook/polymer but my suggestion would be to get rid of HTML imports polyfill and do the steps mentioned in #4958 (comment)

@hipstersmoothie
Copy link
Contributor

@web-padawan Really excited to see this get in

@ndelangen
Copy link
Member

@web-padawan @emilio-martinez @hipstersmoothie @jackblackCH @chhschou @jokin

I'd like to kick this off for realz this time, and get a group together passionate about this.

I'll open a zoom:

Norbert de Langen is inviting you to a scheduled Zoom meeting.

Topic: Storybook for WebComponent (lit-html)
Time: Jun 13, 2019 8:00 PM GMT

Join Zoom Meeting
https://zoom.us/j/510016881

One tap mobile
+17207072699,,510016881# US
+16465588656,,510016881# US (New York)

Dial by your location
+1 720 707 2699 US
+1 646 558 8656 US (New York)
Meeting ID: 510 016 881
Find your local number: https://zoom.us/u/aewm76gu74

Jun 13, 2019 8:00 PM GMT

Hope you can join, feel free to join late, I'll keep it open.

The agenda is:

  • Rename polymer app to something generic or lit-html
  • What are the required changes to make that work
  • Who can do it
  • Who can assist

Screenshot 2019-06-12 at 03 26 49

@shakeandbakeyo
Copy link

just saw this thread. this is exciting! any outcome from the meeting?

@blackfalcon
Copy link

As a user of lit-element and one looking for easy auto documentation solutions, I am very excited to see this addressed ASAP.

@emilio-martinez
Copy link
Contributor

I have been working towards this behind the scenes. The main issue I haven't been able to address fully is HMR support, although some might argue it's a bit of a stretch in the web component space given how 'customElements.define' behaves currently.

I'll try to wrap the work I've done into a PR soon.

@daKmoR
Copy link
Contributor

daKmoR commented Jun 29, 2019

HMR works fine for us in the current implementation :)

what seems to be the problem?

we advocate for splitting class code and customElements.define to be in separate files... so maybe that's why it works for us?

imho this could be a valid requirement if you want HMR

@Ehofas
Copy link

Ehofas commented Jul 15, 2019

Is there any progress on this? We are looking eagerly to try this out!

@kylerberry
Copy link

@Ehofas I too am eagerly awaiting this, but if you're looking to use storybook with lit-element now you can follow the setup at open-wc.

Have you looked at the open-wc tool? they have a feature to use storybooks with lit-element
at https://open-wc.org/demoing/

@tvvignesh
Copy link

@daKmoR Any plan to this? Starting a project with Lit-Elements. So, can I follow the tutorials at open-wc for lit-elements or will something change when this is released? Thanks in advance.

Btw, Amazing work with Storybook! It is something which the community really wanted!

@stale stale bot added the inactive label Aug 10, 2019
@stale stale bot removed the inactive label Aug 11, 2019
@storybookjs storybookjs deleted a comment from stale bot Aug 11, 2019
@web-padawan
Copy link
Contributor

@emilio-martinez any update on this?

This issue is marked as "in progress" so I'm wondering whether there is any actual progress.
I would be glad to help with reviewing and testing, and maybe some suggestions.

@shilman
Copy link
Member

shilman commented Aug 12, 2019

@web-padawan Yes you can see @lonyele 's first draft here #7731

@bennypowers
Copy link
Contributor

Please see #7084 (comment) for a working webpack config, if it's still relevant

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.