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

Using storybook decorators with a "wrapper" custom element in the decorator breaks. #57

Open
rstoneIDBS opened this issue Feb 28, 2024 · 9 comments

Comments

@rstoneIDBS
Copy link

First some background :)

I'm using wc-storybook-helpers to add documentation to a component library that has multiple versions of components, i.e. we have legacy Polymer based components and new Lit based components which live in separate areas of the package. This means that we have multiple components with the same tag name, the consumers of our library are new & old projects - the old continue to use the Polymer components and new one use the Lit components. The plan is to gradually migrate all our apps off the Polymer components but thats some way down the line.

To allow both versions of the components to appear in the same Storybook I've been experimenting with using @webcomponents/scoped-custom-element-registry to scope the newer components via a Storybook decorator, i.e. I create a wrapper component that contains the scoped registry and then register the Lit based components there. This mostly works :)

However I see the following error from wc-storybook-helpers:
Uncaught TypeError: Failed to execute 'observe' on 'MutationObserver': parameter 1 is not of type 'Node'. at wc-storybook-helpers.js:819:48

This is because 'syncControls' is trying to find the custom element tag via a document.querySelector. This obviously fails in my case because the element exists within the shadow DOM of my Storybook decorator wrapper element.

@break-stuff
Copy link
Owner

In those cases do you need the template helper?

@rstoneIDBS
Copy link
Author

In those cases do you need the template helper?

Yes because I still want the props/slots/etc. to update the displayed story.

I have a single wrapper element that I inject the rendered story into via the Storybook decorator. It's a really hacky solution that I don't particularly like, but couldn't figure out a better way.

My wrapper is something like this:

class WrapperElement extends ScopedElementsMixin(HTMLElement) {
  static scopedElements = {
    'my-checkbox': MyCheckbox,
  };

  constructor() {
    super();
    this.attachShadow({ mode: 'open' });
    this.initialised = false;
  }

  set story(story) {
    const temp = document.createElement('span');
    render(story, temp);
    this.shadowRoot.innerHTML = temp.innerHTML;
  }

  connectedCallback() {
    if (!this.initialised) {
      this.initialised = true;
      MyCheckbox.define('my-checkbox', MyCheckbox, this.registry);
      this.shadowRoot.innerHTML = '<my-checkbox></my-checkbox>';
    }
  }
}
customElements.define('checkbox-wrapper', WrapperElement);

And the Storybook decorator:

  decorators: [
    (story) => {
      const renderedStory = story();
      requestAnimationFrame(() => {
        document.querySelector('checkbox-wrapper').story = renderedStory;
      });
      return html`<checkbox-wrapper></checkbox-wrapper>`;
    },
  ],

As above, this is super hacky :) I want to iterate on it as I'm sure there's a better way

@break-stuff
Copy link
Owner

Do you have a single Custom Elements Manifest for both sets of components?

@rstoneIDBS
Copy link
Author

Do you have a single Custom Elements Manifest for both sets of components?

I only generate a CEM for the new components, the Polymer based components are legacy and I don't plan on making any improvements to how they are documented.

@break-stuff
Copy link
Owner

Instead of wrapping the components, can you provide an alternative tag name when in dev or Storybook?

@rstoneIDBS
Copy link
Author

Instead of wrapping the components, can you provide an alternative tag name when in dev or Storybook?

I did try that, but this means the Storybook docs show the wrong component name - something I would rather avoid as I want to make things as easy as possible for my users, it also means I have to come up with some way to pass the modified/correct tag names in depending on what 'mode' (dev/storybook/build) the pipelines are running in.

I'm going to spend a little more time on this today to see what I can come up with.

@rstoneIDBS
Copy link
Author

rstoneIDBS commented Feb 28, 2024

OK, I went back to basics and tried to fix this using just Storybook and now have a much less hacky solution that works :)

I created a new helper function that creates a Storybook decorator to check for mismatching custom element definitions, when it detects this it forces the Storybook frame to reload which allows the "correct" element to then be registered:

export const createCheckForCustomElementDecorator = (tagName, elementClassName) => {
  return (storyFn) => {
    const customElementClass = customElements.get(tagName);
    if (customElementClass.name !== elementClassName) {
      window.location.reload();
    }

    return storyFn();
  };
};

In my use case, all the Polymer components will have a class name of "PolymerGenerated" whereas my Lit based elements have meaningful class names.

To use, its just a matter of adding the decorator to the config for each story:

decorators: [createCheckForCustomElementDecorator('my-checkbox', 'MyCheckbox')],

Would something like this be useful in wc-storybook-helpers?

Edit: I still think the non-hacky solution should involve custom element registry usage, but for now going with the "less code & hacks" version wins it for me

Further Edit: The above works fine in "dev" mode Storybook, however when I build and deploy a static version I get infinite page reloads, so its not quite there yet :(

Further, Further Edit ;) : In my case, the solution was to add the following to my vite.config.js which is only being used for the Storybook build in my case:

    esbuild: {
      minifyIdentifiers: false,
    },

This ensures the class names aren't minified, which was why I was getting the infinite loops.

@break-stuff
Copy link
Owner

This is kind of cool! So you are basically forcing a reload if the component has already been registered. Are you seeing any performance issues because of this?

@rstoneIDBS
Copy link
Author

This is kind of cool! So you are basically forcing a reload if the component has already been registered. Are you seeing any performance issues because of this?

Not really, if you look closely enough you can see the page load and then re-load but for me this is much better than a broken page due to the component not registering. Ideally I would like to get it work using web component registries but this solution (which has undergone a few changes since I first tried it) is the most reliable and requires the least amount of "messing around" :)

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

No branches or pull requests

2 participants