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

Auto migration: Check for missing vite.config.js #25453

Closed
Tracked by #25432
vanessayuenn opened this issue Jan 4, 2024 · 7 comments · Fixed by #25934
Closed
Tracked by #25432

Auto migration: Check for missing vite.config.js #25453

vanessayuenn opened this issue Jan 4, 2024 · 7 comments · Fixed by #25934
Assignees

Comments

@vanessayuenn
Copy link
Contributor

vanessayuenn commented Jan 4, 2024

If a vite.config.js (or any other accepted vite configuration file) doesn't exist, tell the user to run npx vite@latest init because Storybook doesn't set up any plugins by default.

@valentinpalkovic
Copy link
Contributor

What is the context of this? My knowledge is that it is not necessarily required to have a vite.config.js in place. Storybook should support this.

cc @kasperpeulen, @IanVS

@IanVS
Copy link
Member

IanVS commented Jan 5, 2024

We talked about no longer supplying Vite framework plugins like @vitejs/plugin-react ourselves, because it is too much of a maintenance headache when trying to support multiple versions of Vite.

So, if we don't force the user to have a framework plugin configured, how should storybook work?

@valentinpalkovic
Copy link
Contributor

Are you talking about meta frameworks, which don't require a vite config? If you want to use vite with let's say svelte, doesn't a plugin has to be in place and configured? Can you list scenarios where a vite config is not in place and vue, react, svelte,... just still work?

@IanVS
Copy link
Member

IanVS commented Jan 5, 2024

No I'm not talking about metaframeworks.

The reason we added Vite framework plugins like @vitejs/plugin-react ourselves in the past was because we heard from users with design libraries that did not include a Vite config. These libraries were intended to be pulled into a different project and use it's config. But they wanted Storybook to "just work" and were confused why it didn't.

But it's hard for us to know what framework plugin to include, because we support so many versions of Vite now and the plugins have fairly strict peer dependency ranges. The solution we discussed was to remove the "fallback" framework plugins from Storybook, and then create an automigration that would install an appropriate vite framework plugin and either update the user's viteFinal to reference it, or create a .storybook/vite.config.js file and set the viteConfigPath builder option. There is some discussion in https://discord.com/channels/486522875931656193/1023780778439606313/1176806564217241620, but I don't think a final decision was made on which approach to take.

@shilman
Copy link
Member

shilman commented Jan 5, 2024

If we make this change in 8.0 we have to solve for two different cases: (1) upgrades from earlier versions, (2) new installs.

Upgrades. I think the upgrade is pretty easy. We check for what we require as part of an automigration, and then explain to the user what we're going to create for them, and then create it.

Installs. This is the trickier bit. I suppose we could just do something very similar. However I don't like adding a random prompt to the install process. We could consider doing approx the same logic automatically on install.

@valentinpalkovic
Copy link
Contributor

Thanks for the context Ian. Design libraries without an explicit builder configuration were the missing piece for me.

@valentinpalkovic valentinpalkovic changed the title Write automigrations/runtime-check for missing vite.config.js Auto migration: Check for missing vite.config.js Feb 5, 2024
@JReinhold
Copy link
Contributor

  • Do we want to create/extend the configuration file vite.config.js and add the necessary plugins (This would potentially also influence the user's app behavior, if it picks up the config

I think it's dangerous to change any existing Vite config. If a user has a Vite config but without the framework plugin, I can only imagine that is for a very specific reason that we don't want to break. Eg. a Remix-Vite project won't have the React Vite plugin and we wouldn't want to add that there.

Which brings me to my next issue, detecting which Vite plugin is applied is actually pretty hard. They could be applied transitively through other plugins, eg. the SvelteKit plugin includes the Svelte plugin under-the-hood, and the only way to know this is to apply all the plugins via Vite and go through the plugins' names.
So we won't get the full story by just looking at dependencies or imports in the Vite config.

Maybe there's a "good enough" middle ground somewhere though?
Alternatively we could detect the plugins at runtime and show a nice error about it.

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.

6 participants