Storybook HMR not triggering refresh correctly

[deini]

Just generated a component but when I make a change in my component the Storybook doesn't reload.

Looks like its getting the change but I don't see it reflected unless I refresh.

Am I missing something?

[alexlafroscia]

No, not missing anything. This happens to me, too, but I'm not sure why. I'll look into it though.

[deini]

@alexlafroscia I might be totally wrong, but i have a theory that the root of the problem is that the customElement is already defined, so when the define runs again it doesn't replace it.

/shrug

[alexlafroscia]

Hm! That's interesting. I think I remember reading somewhere that there's a way to make customElement.define and HMR playing nicely... I might just not have that set up right. I'll definitely explore that first (and alternatively, if you want to check into things, that would be super cool too!)

[alexlafroscia]

Related: skatejs/skatejs#564

[alexlafroscia]

If that's indeed the issue, I'm not sure about JSX and using a dynamic element name. Since the JSX can't use the constructor anymore in Skate 5, and thus has to use the actual name of the element, I'm not sure how we'd handle not manually setting the name of the component...

[alexlafroscia]

The docs mention using skatejs.define over customElements.define but that's already what we're doing...

[deini]

@alexlafroscia 👋 So I played with this a little bit and 🔎 the issue + docs you mentioned.

define() used to accept an optional name, if name was not present or already registered, it would create a uniqueId() and use that as the name (or append it to the name if name was already registered)

As of right now, 5.x's define() is really simple, and it doesn't deal with name collisions:

I don't have enough context to know if this should be in Skate or not, but for now I added a custom define function used by story.js, something like:

function define(Ctor) {
  const registry = customElements;
  const name = registry.get(Ctor.is) ? `${Ctor.is}-${uniqueId()}` : Ctor.is;

  registry.define(name, Ctor);

  return Ctor;
}

And now I can get Storybook to update when I change my components, the drawback I see is that I'm registering a new element on every change, not sure about the impact of this since its just for dev + storybook.

[deini]

Ahh forgot to mention some really important things:

• I'm not calling define inside of component.js
• Changed the story to use the Component directly in JSX instead of the name.

[alexlafroscia]

Changed the story to use the Component directly in JSX instead of the name.

As I understood it, support for this was being dropped in Skate 5. Is that still the case @treshugart?

I'm not calling define inside of component.js

That's interesting. I'm not sure where define should be called yet. Sometimes it feels like having it the component is nice, because importing the module automatically registers the component. If it's never imported, it's never defined... It feels nice and self-contained. However, as you've mentioned here, there are some drawbacks too.

Where are you calling define instead?

[deini]

Where are you calling define instead?

In story.js and in my app I manually define it, it was not an issue for us since we had a use case where we wanted to be able to rename the component, so we just use customElement.define.

You could do something similar to this skatejs/skatejs#564 (comment) but its not not exactly how you want it, you would still need to invoke it :/

[alexlafroscia]

Hm, that's interesting. Kind of lets the importer decide if they want to auto-define or not, right?

What do you think a good default behavior for the Yeoman generator would be?

[deini]

Kind of lets the importer decide if they want to auto-define or not, right?

Correct.

What do you think a good default behavior for the Yeoman generator would be?

Hmm, I think the generator should aim for simplicity, that is auto defining the element on import (so no changes to the current implementation). However at least in my use case having HMR is pretty useful, but I can't think of a solution right now that would solve for both cases.

Feel free to close the issue.

[deini]

@alexlafroscia I had more time to play around with this and ended up needing the element to auto-define.

What I ended up doing is having 2 files, one exporting the class of the Element and another one that includes it and defines it. I use the latter when exporting it as a library, but for my story.js I'm using the former.

This way I can get storybook to update and have my elements auto-defined when requiring them in other apps. However this still relies on sing the class directly in JSX instead of the name.

[treshugart]

@deini if you want to send a PR I'm happy to review it.