Svelte-Vite: Upgrading from 6.5 to 7.0 breaks Storybook because of svelteOptions in main.cjs

[JReinhold]

Relevant maintainers:
@IanVS @benmccann @joshwooding

tl;dr
The default svelteOptions in main.cjs from 6.5 are breaking Storybook 7.0. I propose to not support that config property anymore, since it's unnecessary now that we are automatically loading the project's options from the root Vite config and Svelte config.

The Problem

In a 6.5 setup with Svelte and Vite, the svelteOptions property in main.cjs is used to add specific options to the Svelte Vite plugin, most often adding the preprocess from svelte.config.js (which is the default content after sb init).
In 6.5 this is needed because we didn't load the root Vite config automatically.

The default svelteOptions in 6.5 looks like this:

svelteOptions: {
	preprocess: require("../svelte.config.js").preprocess,
},

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.

The require call is what's causing us problems, it makes Storybook fail instantly with:

ERR! SyntaxError: Identifier '__esbuild_register_import_meta_url__' has already been declared

Most likely because Svelte and/or SvelteKit have moved to pure ESM, which we can't import with require (changing it to import doesn't fix it though, so it might be caused by something else).

In 7.0 we automatically load the user's Vite config, and by extension Svelte config, so this isn't necessary anymore - users can and should remove their svelteOptions property unless there's a very very specific reason for them to have it there

Proposed Solutions

A. Automigration to remove svelteOptions

I think at least we should have an automigration that removes the svelteOptions property from the configuration. Initially I thought it should only suggest removing it if it was the default options, but after my GitHub crawling I personally think we should always recommend removing it, with a prompt explaining why, and why not.

This solution could be done with B or C too.

B. Remove support for svelteOptions altogether

It would technically be a breaking change to not support svelteOptions which we want to avoid at this stage in the release cycle, however:

1. No open source users are using it (closed source could though)
2. Our current 7.0 beta actually doesn't pass the options along to the Svelte Vite plugin, so we are already not supporting it, but I think that's not intentional, it was just forgotten in Vite: Add vite framework plugin if not found #19259.
3. Default options are already supported by just loading the user's Vite config.
4. For differentiated options there's still a workaround for the user, by mutating the Vite config in viteFinal and thereby replacing the Svelte Vite plugin with their own instance and options.

C. Keep support for svelteOptions

We could keep supporting svelteOptions. It would require that if we detected custom svelteOptions, we'd remove the Svelte Vite plugin added by the root Vite config, and add our own instance with the user's custom options.

Additional Context

@IanVS have already flagged this before in #19280, but that issue and work ended up diverging in to so many topics that we forgot to conclude anything from it.

To Reproduce

yarn create svelte-with-args --name=svelte-options-bug --template=skeleton --types=null --no-prettier --no-eslint --no-playwright --no-vitest # SvelteKit project
cd svelte-options-bug
npx sb init # add Storybook 6.5
npx sb@next upgrade --prerelease # upgrade to 7.0
yarn storybook # see it fail

[shilman]

@JReinhold FWIW it’s not a problem to have breaking changes in the Beta or even the RC. We try to handle all of them in the alpha, but stuff always comes up. What’s important is that we ship the right thing for users

[IanVS]

In 6.5 this is needed because we didn't load the root Vite config automatically.

That's not quite true I think. IIUC, the svelte config is normally defined in a svelte.config.js file. @benmccann made a change a while back in 6.5 which reads this file in the vite builder, for the most part making svelteOptions unnecessary in 6.5 as well. We kept the option, though, in case users wanted to customize the svelte config for Storybook. It sounds like you did some research, though, and found that it's not being used in this way.

In 7.0 we automatically load the user's Vite config, and by extension Svelte config

We also have separate logic to load the svelte config, using loadSvelteConfig from '@sveltejs/vite-plugin-svelte', in order to configure the svelteDocgen plugin.

---

I'm in favor of dropping support for svelteOptions within main.js for vite projects, given your exploration of existing usages and the fact that viteFinal exists as an escape hatch. An automigration to explain this and remove the field would be gravy.

What about webpack, though? It also supports specifying svelteOptions, and it doesn't look like it has the same logic to automatically read svelte.config.js that we have in the vite builder.

[benmccann]

I'm supportive of removing svelteOptions. The webpack version should probably read svelte.config.js as well. On the Svelte side, for webpack we have svelte-loader which is quite ignored and barely maintained, so it doesn't read svelte.config.js yet, but we should probably make it do so as well

[JReinhold]

Thanks for clearing that up @IanVS, that was a misunderstading on my part.

Let's remove support for svelteOptions in Vite-based projects, and keep it in Webpack based. If/when svelte-loader automatically picks them up we should remove support for svelteOptions altogether to have feature parity.

[shilman]

Ta-da!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.0.0-beta.8 containing PR #20270 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

Closing this issue. Please re-open if you think there's still more to do.

[yustarandomname]

My threlte project requires a special preprocessor. How can I change the preproccesors, with the removal of svelteOptions?

Instance of a threlte svelte.config.js file:

import preprocess from 'svelte-preprocess'
import seqPreprocessor from 'svelte-sequential-preprocessor'
import { preprocessThrelte } from '@threlte/preprocess'

const config = {
	// …
	preprocess: seqPreprocessor([preprocess(), preprocessThrelte()])
}

export default config

[benmccann]

@yustarandomname as long as you have it in svelte.config.js like that then storybook will read it from there and use it, so you don't need to specify anything in svelteOptions