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

Add sveltekit plugin in .storybook/main.cjs #5

Closed
michaelwooley opened this issue Jun 30, 2022 · 5 comments · Fixed by #7
Closed

Add sveltekit plugin in .storybook/main.cjs #5

michaelwooley opened this issue Jun 30, 2022 · 5 comments · Fixed by #7

Comments

@michaelwooley
Copy link
Owner

An important area where the plugin is supposed to help!

Related: sveltejs/kit#5184 (comment)

Ability to get rid of separate svelteOptions config?

Also see whether adding the plugin removes the need to separately specify items like the preprocessor:

https://github.com/michaelwooley/storybook-experimental-vite/blob/ba4c8a830baa10c0024ecc0c0f9376cc3f2d38a7/.storybook/main.cjs

@IanVS
Copy link

IanVS commented Jun 30, 2022

This is a good question for @benmccann, but my understanding is that the vite plugin is only for options that would normally have been specified in the vite portion of svelte.config.js. As such, I think that the svelteOptions are still needed, since those are provided to @sveltejs/vite-plugin-svelte, and aren't really related to sveltekit, but rather svelte itself. But I could well be wrong here, I'm not terribly familiar with svelte or sveltekit.

@benmccann
Copy link
Collaborator

Thank you so much for testing this!

I would expect the svelteOptions to live in svelte.config.js. SvelteKit and vite-plugin-svelte will both read that file. If the Storybook Svelte plugin needs those options it would probably be nice to have it read that file as well to avoid making the user specify the options twice

I'm also a bit surprised that you need to specify viteFinal. I'm not that familiar with that option and how it works, but I would expect that the SvelteKit Vite plugin would be providing those aliases for you so that you don't need to specify them again

@benmccann
Copy link
Collaborator

benmccann commented Jun 30, 2022

Btw, feel free to hop onto Discord if you'd like faster responses. We could coordinate in the Vite server since I know both Ian and myself are on that one. I'll start coordinating with him in the #storybook channel. Hopefully we can figure out the alias issue.

I thought I'd answer the open questions from the readme:

Vite 3 support? Is it important? Do we need to pin to vite 2 for now? (storybookjs/builder-vite#394)

Neither Storybook nor SvelteKit support it yet though we're both testing against it. For now, there's no need to support it and I wouldn't worry about it, but SvelteKit will switch to it once the final release is out, which will probably be in the next week or two. On the SvelteKit side I don't think you'll have to do anything except bump the SvelteKit version.

Importance of adapters? I don't think that adapters matter at all for what shows up in the .svelte-kit/runtime dir, right? Only the contents of the .svelte-kit/build dir are affected?

Don't worry about adapters at all. Neither dev nor preview use them. They're only for when you deploy to production. The location of the build output for adapters varies by adapter.

@michaelwooley
Copy link
Owner Author

Thanks for all of this info @benmccann and @IanVS!

After work I'll try #7, write an actual reply, and hop onto the discord.

If you find this to be a useful test project I'd be happy to give you merge permissions, etc. so that you don't have to wait on me. Whatever works for you. (I don't have any grand plans for this repo: It is only meant to test this out.)

@benmccann
Copy link
Collaborator

Thanks @michaelwooley! I'd be happy to have merge permissions. I'd love to get this repo in a good state and then create a Svelte Adder for Storybook. Once we have that then we probably won't need this repo anymore, but it's a nice place to get something working as the basis of an Adder. If you could either merge #7 or give me permission to, that would be great!

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.

3 participants