[go: nahoru, domu]

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

Svelte HMR broken in stories and components #52

Closed
IanVS opened this issue Jun 28, 2021 · 13 comments · Fixed by #362
Closed

Svelte HMR broken in stories and components #52

IanVS opened this issue Jun 28, 2021 · 13 comments · Fixed by #362

Comments

@IanVS
Copy link
Member
IanVS commented Jun 28, 2021

Changes to story files and component files do not appear in storybook until a full page refresh.

Ref: #3 (comment) and confirmed from another svelte user in discord.

@eirslett
Copy link
Collaborator

I suspect this was fixed in #53 for Svelte as well? I'll close this issue, if it's actually fixed then we could reopen the issue.

@Enteleform
Copy link

@eirslett I'm still experiencing this issue in 0.0.12

@eirslett eirslett reopened this Aug 1, 2021
@IanVS
Copy link
Member Author
IanVS commented Feb 17, 2022

Some fixes for svelte were just released. Would folks here like to try out https://github.com/eirslett/storybook-builder-vite/releases/tag/v0.1.16 to see if hot reloading is improved for you, and report back? Thanks!

@vish01
Copy link
vish01 commented Apr 28, 2022

I'm using latest versions of builder-vite, svelte, @storybook/svelte and can confirm this is still an issue only for stories. When I make a change in a svelte component, HMR seems to be working fine.

@IanVS
Copy link
Member Author
IanVS commented Apr 28, 2022

Thanks for mentioning this, @vish01. I confirmed it's still an issue in our svelte example. I think it's similar to the problem we had with HMR for react components, where we needed to exclude the story files from being HMR boundaries. I've opened a feature request with vite-plugin-svelte to hopefully add an option that lets us customize the HMR boundaries similar to react.

For now, the only workaround I can think of is to provide this in your svelteOptions of .storybook/main.js:

  svelteOptions: {
    preprocess: preprocess(),
    hot: false, // <-- cause page reloads all the time
  },

EDIT You shouldn't need this ^ workaround with version 0.1.33 or higher.

@IanVS IanVS closed this as completed in #362 May 1, 2022
IanVS added a commit that referenced this issue May 1, 2022
Fixes #52

Svelte story files have the same issue as react story files.  The webpack HMR seems to prevent vite's HMR from working correctly, so we need to exclude story files as HMR boundaries in vite.  The svelte plugin doesn't expose as nice of a way to do this, but thanks to a suggestion from @domyen in sveltejs/vite-plugin-svelte#321 (comment), I was able to make it work.

To test:
Start up the svelte example, edit a `.stories.svelte` file, and confirm that the page reloads and shows your changes.
@IanVS
Copy link
Member Author
IanVS commented May 2, 2022

@vish01 Can you please try out the latest version https://github.com/storybookjs/builder-vite/releases/tag/v0.1.33 and let us know if this solves the issue for you?

@vish01
Copy link
vish01 commented May 2, 2022

Hey @IanVS. I just tried out the 0.1.33 version, it kinda acts like the hot:false option(The UI for the component reloads, not the entire page), but I still don't see HMR work for stories. It still works for changes made in components though.

@IanVS
Copy link
Member Author
IanVS commented May 2, 2022

Hi thanks for checking it out. Unfortunately, that's the best we can do right now, and it's the same experience in react as well. Component changes can HMR, but story file changes require a refresh, due to the storybook manager also watching story files for HMR of the sidebar (list of stories).

At least the stories update without requiring a page refresh now, so hopefully a bit better than before.

@vish01
Copy link
vish01 commented May 2, 2022

Stories do refresh when using the latest version of builder-vite, but code changes made to stories still doesn't work until I reload the page myself or set the hot:false option. I'd still keep this issue open since HMR is still broken for stories or open a new issue if we want to separate our stories from components since it works fine with components.

@IanVS
Copy link
Member Author
IanVS commented May 2, 2022

code changes made to stories still doesn't work until I reload the page myself or set the hot:false option.

Oh I see, I misunderstood what you were saying earlier. Any chance you could provide a minimal reproduction so I can troubleshoot further?

@IanVS IanVS reopened this May 2, 2022
@vish01
Copy link
vish01 commented May 2, 2022

@IanVS While creating repro, I found that the issue seems to be happening only when you modify Tailwind classes as props in stories. If I modify normal props like string, object, those seems to be working fine. Here's the repo since you wanted a minimal repro:
Storybook HMR repro
When you serve storybook, modify Card.stories.svelte and if you modify any of the justifyXXX props, you'll see it doesn't gets reflected in Storybook UI, but if you choose from the controls within the UI, it will change.

@IanVS
Copy link
Member Author
IanVS commented May 4, 2022

Thanks for explaining. I've tested out your example, and I find that as long as I don't change any of the controls in the story, I can edit any of the args, including the justify args, and they are reflected without having to refresh the page. However, if I change a control, and then try to also change the arg in the story file, then things break. But, this also happens in the webpack builder too, so I think it's a bug within storybook itself.

I also see that query params are being added to the url like &args=justifyHeader:right;justifyTitle:!undefined;justifyDescription:!undefined, which there's a PR to fix in storybookjs/storybook#17973. That takes care of the !undefined issue, but it seems that it still won't be possible to update story args that have been put into the url (in both webpack and vite). I don't see any open issue for that. If you're willing to open one, pointing to the reproduction repo, that would be excellent.

I'm going to re-close this issue for now, but if you think this is a vite-builder specific issue, please let me know. Thanks again for bringing this up, and for the reproduction steps.

@IanVS IanVS closed this as completed May 4, 2022
@vish01
Copy link
vish01 commented May 4, 2022

Sounds good, I'll create a new issue on Storybook then. Thanks on checking this for me!

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

Successfully merging a pull request may close this issue.

4 participants