fix: remove svelte-options from svelte-vite

[benmccann]

What I did

Removed two unnecessary pieces of functionality. We don't need to add vite-plugin-svelte as it will always be present of the user's app won't work. We also don't need to add svelteOptions as I believe this was just a holdover from before we loaded the svelte.config.js file

Checklist

• Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
• Make sure to add/update documentation regarding your changes
• If you are deprecating/removing a feature, make sure to update
  MIGRATION.MD

Maintainers

• If this PR should be tested against many or all sandboxes,
  make sure to add the ci:merged or ci:daily GH label to it.
• Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

[JReinhold]

I agree with removing the svelteOptions but I'm not so sure about not adding vite-plugin-svelte. We have this pattern for all our Vite-based frameworks, and I can think of a few scenarios where it will still be needed:

1. Users building UI libraries without any app, but just a Storybook and a custom build configuration. They won't have a vite.config file, and thus we need to add the minimal here.
2. Users with custom Vite config paths that we don't automatically detect, so there won't be a default Vite config to extend from.

Maybe @IanVS has some background knowledge here, since this is seen in most of the Vite-frameworks.

[benmccann]

Users building UI libraries without any app, but just a Storybook and a custom build configuration. They won't have a vite.config file, and thus we need to add the minimal here.

That's not really possible. Even if you're building a library and not an app, you still need a vite.config.js

Users with custom Vite config paths that we don't automatically detect, so there won't be a default Vite config to extend from.

That would cause a world of pain. We'll be missing the SvelteKit plugins and all of the user's configuration. It's better that we address that rather than cover it up. Perhaps if we don't see a vite.config.js we print a warning that the user should modify the package.json to insert --config custom-vite.config.js or whatever as appropriate.

[IanVS]

Users with custom Vite config paths that we don't automatically detect, so there won't be a default Vite config to extend from.

There is now a builder option to specify the path to a custom vite config. It's a good idea to add a warning message with instructions, I'll look into that.

I think for sveltekit specifically, it makes sense to remove the requirement for the plugin since as @benmccann said, it's guaranteed that the user has it if they're using sveltekit. We can also re-evaluate if we hear use cases from users who don't have it somehow.

[JReinhold]

@IanVS but this change in on the svelte-vite framework, do you think it's safe to remove it there? And if so, should we remove the similar check for all the other Vite-based framework?

[IanVS]

Oh sorry, I was on my phone and assumed it was sveltekit since it was from @benmccann and he was talking about sveltekit.

Even if you're building a library and not an app, you still need a vite.config.js

That's not true in all cases. We do indeed find a good number of folks building, for example, react components without any vite config, which is handled by the consuming apps. Our very own @joshwooding maintains a library like this. I suppose the same thing can be done in svelte projects.

I guess this is a downside to extending sveltekit from svelte-vite. I wonder if we're better off duplicating some code and decoupling the two.

[benmccann]

Ah, so we can have Svelte libraries that don't do any bundling and in that case you should still be able to run Storybook. Does Storybook always need a bundler? It looks like it adds @storybook/svelte-webpack5 by default, which is not what we'd want as webpack is persona non grata in Svelteland. If we're going to create a vite.config.js for the user because they don't have one and we need one to run Storybook then it makes sense to add the Svelte plugin to it. I don't think we should need or want to mess with an existing vite.config.js.

[IanVS]

Right, Storybook does always need a bundler. So it's possible that someone would run npx sb@next init --builder=vite to get their storybook running with the vite bundler even if they don't have a vite config file.

It looks like it adds @storybook/svelte-webpack5 by default, which is not what we'd want as webpack is persona non grata in Svelteland.

It's still possible to use svelte with webpack, right? I do wonder if we could make a way to invert the default for svelte though, and default to vite and only use webpack if we detect a webpack config. That might be tricky based on the way the CLI works, I'd have to look.

If we're going to create a vite.config.js for the user because they don't have one

We don't actually create a file, but we do create a vite config, that we provide to the vite server when it starts up or when we build for production.

I don't think we should need or want to mess with an existing vite.config.js.

It sounds like your suggestion is that instead of adding the plugin if the plugin isn't found, to add it if a vite config is not found. I think I like that, since it addresses the case we're trying to handle, though I need to think about how we can accomplish it.

[IanVS]

For now, what do you think about leaving it the way it was, and updating this PR to just remove svelteOptions? We can address the rest separately.

[benmccann]

Yeah, agreed. I sent #20966 regarding the default for new project creation

[IanVS]

We also don't need to add svelteOptions as I believe this was just a holdover from before we loaded the svelte.config.js file

So, just to confirm what this is doing. We previously kept the option around so that users could override their app's svelte config for Storybook. We were loading their svelte.config.js file, and then spreading in overrides from svelteOptions. I don't think we ever came up with a use case where this was actually necessary, though. In case someone does need a different svelte config in storybook, is there still a way that they can accomplish that? Can anyone think of a reason someone might need to do that?

[JReinhold]

It sounds like your suggestion is that instead of adding the plugin if the plugin isn't found, to add it if a vite config is not found. I think I like that, since it addresses the case we're trying to handle, though I need to think about how we can accomplish it.

I'm honestly fine with either, but I think whatever we do, our arguments are not specific to Svelte, so it should be done across all the Vite frameworks for consistency.

---

svelte.config.js file, and then spreading in overrides from svelteOptions. I don't think we ever came up with a use case where this was actually necessary, though.

I'll refer to my investigation I did in #20236, specifically:

In theory it could also be used to add any options that the user wanted to be different from their app Svelte configuration, however after inspecting all instances of svelteOptions found on GitHub, exactly 0 of them had options that differed from their main Svelte config. Based on that, we can maybe conclude that that is actually not a use case.

---

... In case someone does need a different svelte config in storybook, is there still a way that they can accomplish that?

The svelte() export from vite-plugin-svelte takes in inlineSvelteOptions as overrides, so I think users could - in their viteFinal - remove the vite-plugin-svelte plugin and add their own with inline options if they wanted. Not very ergonomic, but doable.

[IanVS]

I think whatever we do, our arguments are not specific to Svelte, so it should be done across all the Vite frameworks for consistency.

Agree 💯

[IanVS]

This comment isn't exactly accurate for right now, though it hopefully will be sometime in the future, right?

[benmccann]

You said above:

We don't actually create a file, but we do create a vite config, that we provide to the vite server when it starts up or when we build for production.

I was assuming that this in-memory Vite config would be created whether or not a vite.config.js is present. I suppose the old comment might be a bit more accurate in that it applies either way whether or not that file is present. But I think my comment better describes the use case we're trying to address and when we might encounter it, which is perhaps more useful for folks who are editing this code.

[IanVS]

Sure, I was being pedantic. They could have a vite config without the plugin, and we would add it. I'm just splitting hairs.