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

SvelteKit: Automatically support Kit-specific features #20239

Merged
merged 10 commits into from
Jan 12, 2023

Conversation

JReinhold
Copy link
Contributor

@JReinhold JReinhold commented Dec 13, 2022

This PR keeps the vite-plugin-sveltekit-setup that handles many SvelteKit-specific features, so they get supported out of the box in Storybook.

Currently this PR is blocked by the issue that built Storybooks will always show the "No story" warning on initial load/reload. Navigating between stories afterwards works, and everything works fine in dev.
This issue is also causing the iframe viewmode to be broken.

Once that has been fixed, this should be good to merge.

To test this out:

  1. git checkout origin/sveltekit-keep-compile-plugin
  2. yarn task --task sandbox --template svelte-kit/skeleton-js --start-from install
  3. cd sandbox/svelte-kit-default-js
  4. yarn build-storybook
  5. npx http-serve storybook-static

@JReinhold JReinhold added maintenance User-facing maintenance tasks svelte labels Dec 13, 2022
@JReinhold JReinhold requested a review from shilman December 13, 2022 23:21
@JReinhold JReinhold self-assigned this Dec 13, 2022
@JReinhold JReinhold removed the request for review from shilman December 14, 2022 12:43
@ndelangen
Copy link
Member

Is this part of the 7.0 beta project? What column does this belong in?

@ndelangen
Copy link
Member

I had to allow for vite version 4, because that's what is used in sveltekit, it seems.

I also extracted that into it's own PR: #20294

@ndelangen
Copy link
Member

@JReinhold I do not see the situation you describe happening.

Maybe there's a step missing in your steps to reproduce?

I ran your steps on this branch and got this static-storybook:
storybook-static.zip

I do not see the "no story" UI show up"
Screenshot 2022-12-15 at 20 48 00

@ndelangen
Copy link
Member

what is making unit tests fail?!?!

@benmccann
Copy link
Contributor

I just tested and it worked fine for me as well. I did run into a separate issue: #20595. But I didn't see the "no story" UI, so I think this could be merged after being rebased

npx http-serve storybook-static

I wouldn't have known this was a thing you could do without reading the PR description. Should we add it to the package.json? Or the readme?

@JReinhold
Copy link
Contributor Author

I just tested and it worked fine for me as well. I did run into a separate issue: #20595. But I didn't see the "no story" UI, so I think this could be merged after being rebased

Alright, then let's clean it up and merge it, see if it was just my setup that was bad. Did you try having a story that actually imported and used all the supported SvelteKit modules? I wonder if that could cause it, as I had such a story. (I'll see if I can dig it out from somewhere)

npx http-serve storybook-static

I wouldn't have known this was a thing you could do without reading the PR description. Should we add it to the package.json? Or the readme?

I think this is generally documented a few places in the Storybook docs, mainly here: https://storybook.js.org/docs/react/sharing/publish-storybook
I don't think we need to specifically point this out in relation to SvelteKit then? But the SvelteKit docs could do a better job of pointing the user to the Storybook docs though.

@benmccann
Copy link
Contributor

Did you try having a story that actually imported and used all the supported SvelteKit modules?

No, I didn't. It's possible that could be the cause

@benmccann
Copy link
Contributor

@JReinhold I just did import { dev } from '$app/environment'; and included the value in Button.svelte and it worked fine. I don't know if there's another value you were importing that might have caused issues. This seems like it's probably in a good enough place that we can merge it as is and fix any bugs in followups

@benmccann benmccann marked this pull request as ready for review January 12, 2023 22:08
@JReinhold JReinhold merged commit 039e909 into next Jan 12, 2023
@JReinhold JReinhold deleted the sveltekit-keep-compile-plugin branch January 12, 2023 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks svelte
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants