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

[Feature] Add Vite-Builder Support #64

Closed
Gaubee opened this issue Jul 14, 2022 · 8 comments · Fixed by #72
Closed

[Feature] Add Vite-Builder Support #64

Gaubee opened this issue Jul 14, 2022 · 8 comments · Fixed by #72
Labels
bug Something isn't working

Comments

@Gaubee
Copy link

Gaubee commented Jul 14, 2022

Vite-Builder is one of the office builder, pls add support.❤️

@Gaubee Gaubee added the bug Something isn't working label Jul 14, 2022
@benmccann
Copy link
Contributor

I think there's already support. You can see an example project here:

https://github.com/michaelwooley/storybook-experimental-vite

@konstantinblaesi
Copy link

@benmccann when doing

npm install
npm run storybook

for that repository, I see traces of webpack. Is storybook somehow ditching the vite builder once this package is installed/enabled/used?

@benmccann
Copy link
Contributor

It uses Vite, but is still pulling in webpack anyway. Should hopefully be fixed in Storybook 7

@IanVS
Copy link
Member

IanVS commented Sep 23, 2022

That's right, in Storybook 6, webpack is still used to build the "manager" part of the UI, which no longer happens in SB 7.

@IanVS
Copy link
Member

IanVS commented Oct 4, 2022

@j3rem1e what would you think about adding vite-builder support into this addon? Currently, we have a custom vite-plugin in the svelte-vite framework (which I realized recently isn't being used, though it is in 6.5), which imports some files from this addon and duplicates some of what this addon is doing.

What would you think about making this addon-svelte-csf package a bit more general, including the vite plugin here, and adding a viteFinal alongside the current webpack config? It would mean a few more optional peer dependencies perhaps, but it might be nice to have the related logic all in one place, and one addon to tell people to install. What do you think? Should I work on a PR?

@j3rem1e
Copy link
Contributor

j3rem1e commented Oct 4, 2022

@IanVS yep, that's a good idea ^^

IanVS added a commit to storybookjs/storybook that referenced this issue Oct 5, 2022
Typescript-based svelte components did not generate docs correctly, and svelte-native stories were not possible.

## What I did

In the transition to 7.0, we neglected to load svelte options from `svelte.config.js` (and `svelteOptions` from storybook overrides) and pass them to the svelte docgen plugin, which means that svelte components written with typescript did not work correctly.

Also, while we brought over the `csfPlugin` for svelte, we never actually used it.  So, svelte-native stories would be broken even if `@storybook/addon-svelte-csf` was added.  The fix here is a bit of a temporary one, hopefully.  It uses the same strategy from the 6.5 vite-builder, but it would be better if we could add vite-support to the addon directly, which I've suggested in storybookjs/addon-svelte-csf#64 (comment).

## How to test

Currently we do not have a svelte-vite typescript sandbox, nor do we have any svelte-native stories.  But, I bootstrapped my own project, and copied over the `dist` folder from the `svelte-vite` framework in this branch, and confirmed that both docgen and svelte-native stories do not work without these changes, and do work with them.
@IanVS IanVS mentioned this issue Oct 6, 2022
@benmccann
Copy link
Contributor

#72 was merged and said it closes this issue. I'm confused as to why GitHub didn't auto-close this issue. Maybe it was just a GitHub glitch? Should this issue be closed or is there more to do here?

@IanVS
Copy link
Member

IanVS commented Aug 23, 2023

I think the problem is that I used the storybook monorepo convention of issue: #xyz instead of closes: #xyz, and the same scripts that run there, don't run here. I do believe this can be closed now, thanks for mentioning it.

@IanVS IanVS closed this as completed Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants