Automatically add svelte libraries to vite.optimizeDeps.exclude

[dominikg]

Describe the problem

dedupe does not work for optimized dependencies and this breaks stuff.

See
#124
vitejs/vite#3910
https://github.com/bluwy/vite-svelte-dedupe

Describe the proposed solution

To avoid this problem, automatically add svelte libraries to optimizeDeps.exclude.

Finding those libraries isn't bulletproof, the best you can do is check for the existance of a "svelte" field in it's package.json,

see https://github.com/sveltejs/kit/blob/69b92ec16ce92b86bc5a540daa8ddbd84ba400ef/packages/kit/src/core/utils.js#L85

Alternatives considered

keep the status quo and users have to apply it manually

Importance

would make my life easier

[mattjennings]

I like this. It's worth noting that svelte-kit package does not add a pkg.svelte property which is unfortunate, but it does add the .svelte files to pkg.exports. Maybe we could check those as well?

[dominikg]

it does? then it would also have to check that field in the code I linked 🤔
thanks for pointing it out.

also vite-plugin-svelte and other bundler plugins would also need to make sure to resolve via that custom ext.

[mattjennings]

That would explain why my libraries using kit package only work when under dependencies. Otherwise, they get that Must use import to load ES Module message unless it's added to vite.ssr.noExternal manually

For reference, here's the package.json: https://unpkg.com/svelte-modals@1.0.3/package.json.

[dominikg]

ah, it's only adding the path mapping, not a custom field. that's OK.

your problem is likely the incompatibility with esm-only packages in vite SSR.
vitejs/vite#4230 (comment)

[mattjennings]

Oh, yes, sorry, I meant to say we could skim through pkg.exports with .svelte extensions as an alternative to looking for pkg.svelte if it's not there. There is an issue being discussed for kit to add pkg.svelte, so maybe this is more of a kit issue anyway.

Thanks for the heads-up on the esm-only SSR bug though, that looks like the culprit.

[benmccann]

@btakita also reported that Vite optimization breaks Svelte in vitejs/vite#4306

But skipping optimization seems like a terribly painful solution because I believe it will also skip optimizing the transitive dependencies of the dependency you specify? Assuming that's right, it means that you cannot use any Svelte component with a non-ESM dependency, which would be a huge pain. And @GrygrFlzr suggested we can't simple re-include those transitive dependencies to be optimized vitejs/vite#3961 (comment)

I'm not in favor of short-term workarounds because it makes it harder to submit bug reports for the true underlying issues, etc. I've been having a hard time because many libraries I find hit multiple issues, which makes it hard to untangle things and come up with good test cases. I'm afraid this would just add an additional layer of complication. I've started creating a list of the various issues we need to solve: sveltejs/kit#2086

[benmccann]

I was thinking about this some more, dependency pre-bundling should only happen for non-ESM libraries and most Svelte components should be written in ESM. I wouldn't expect to hit this that often as a result. But could we instead try to detect Svelte components that aren't written in ESM and print a warning that the package source should be converted or that they should distribute an ESM version?

[dominikg]

Printing a warning is a good thing, asking users to start pestering library authors/maintainers to use a modern template like sveltekit package or follow it's "distribute as esm, include preprocessed .svelte" pattern

Unfortunately most older libraries are based on https://github.com/sveltejs/component-template or variants of it, so there are many different patterns out there.

As long as dedupe is only working for unoptimized dependencies, i think we should default to exluding all svelte libraries. If that excludes a cjs library that stops working as a result, the printed warning helps and users can still decide to manually reinclude it via optimizeDeps.include.
I prefer this over continued bug reports because failed deduplication, but happy to discuss this further :)

[benmccann]

sveltejs/component-template is also ESM, so there really shouldn't be many non-ESM Svelte components out there. I can't recall ever seeing a non-ESM Svelte component. I tried following the links to the various issue reports about this and the only instance I see is of someone manually adding a library to optimizeDeps.include. Perhaps we should just check that config value and print a warning if there are any Svelte components in there

[dominikg]

given this lib

  "name": "some-svelte-lib",
  "svelte": "src/index.js", // export { default as default } from './Component.svelte';
  "module": "dist/index.mjs", // esm  bundle via rollup, contains compiled svelte component
  "main": "dist/index.js", // UMD via rollup, contains compiled svelte component
  "type": "commonjs" // not actually in package.json, but missing type: module means commonjs by default

and this svelte component in a vite project

<script>
import Foo from 'some-svelte-lib'
</script>
<Foo/>

vites optimizer is going to auto optimize 'some-svelte-lib' unless told not to. The fact that main and svelte point to different things doesn't make it easier. and neither that depending on the project of the user main or module are going to be used.

I tried to have a look at the implementation of the optimizer but now my head hurts. its a mix of esbuild plugins and the plugincontainer resolver.

[bluwy]

I doubt Vite will be fixing this upstream as it's a tricky issue to tackle, so I'm on board with implementing this in the meantime. @benmccann did brought up a good point though:

And GrygrFlzr suggested we can't simple re-include those transitive dependencies to be optimized vitejs/vite#3961 (comment)

Because currently there's two steps to this issue:

1. Issue with svelte library, put it in optimizeDeps.exclude
2. Oh no, that svelte library imports some CJS-only library, install that library (if pnpm) and put it in optimizeDeps.include.

With this change, we're solving step 1, and the user would only need to care of step 2. But the jump would be confusing for a lot of users. "Why micro-manage libraries I'm not directly using?".

It'sa bit of a double-edge sword. Getting people to understand step 1 helps with step 2, but then they would wish to automate step 1. We may need better docs to clarify the quirk then.

[benmccann]

given this lib ... vites optimizer is going to auto optimize 'some-svelte-lib' unless told not to

I think it will today. But I expect that will not be the case in the future. We're talking about having Vite use CJS deps directly without any bundling

Issue with svelte library, put it in optimizeDeps.exclude

For most issue with Svelte libraries you don't have to do this. I've created a list of all the issues I see people hit using SvelteKit and Vite here: sveltejs/kit#2086

They almost all will be solved by just loading CJS deps directly on the server and Vite not doing bundling and optimization on the server by default and I think there's mostly agreement on doing that as the path forward.

We've got a plan to fix the issue for real. I'd prefer to focus on that rather than layering on additional changes to make Svelte more different from Vite which will make diagnosing and fixing issues more difficult. If the solution proposed here were the desired solution I think it at least should be applied in Vite for all "external" files and not in this plugin, but it really feels like more of a workaround to me

[dominikg]

if vite really doesn't consider dependencies of excluded dependencies for optimization that could be a bug.
they won't drop optimization alltogether as stuff like lodash would blow up the initial load with a lot of requests.

the whole separate esbuild optimizer without a way for plugins to hook into is suboptimal. i'd rather have it use the build plugin transfom chain or a filtered chain via apply: "optimize"

[benmccann]

they won't drop optimization alltogether as stuff like lodash would blow up the initial load with a lot of requests.

we're only talking about dropping optimization for the server side and I believe the issue with lots of requests would only be on the client side

[benmccann]

I dug into this some more to better understand it (#135 (comment)). Basically Vite is just skipping any file that's not .js when pre-bundling and so you get a half-bundled package which leads to the bug vitejs/vite#3910.

I also found out that we're basically already adding everything (not just .svelte but everything) to optimizeDeps.exclude in SvelteKit: sveltejs/kit#759. Though I sort of want to revert this change because otherwise you can never use Svelte components with CJS dependencies. We could include just the Svelte components in exclude as proposed here, but then I wonder about the pnpm issue. I don't quite understand what that issue is. Could you shed some more light on it?

[bluwy]

Yeah, I probably should update vitejs/vite#3910 with a better description. I realized now most info is in the repro instead 😅

Re pnpm issue, if we have a svelte-abc library, and it imports xyz library which is CJS only, we currently need to add xyz to optimizeDeps.include, so Vite is able to convert the CJS to ESM. But since pnpm is strict by default, doing so wouldn't work since you can only import dependencies you declare in package.json. So the way around it would be to either (1) set pnpm shamefully-hoist=true or (2) install that dependency in package.json so we can import it.

I only found out about (1) yesterday which could help a alleviate micro-management of these transitive dependencies, but it defeats on of the cool feature of pnpm.

[dominikg]

pnpm actually added another cool feature to update package declarations selectively without resorting to shamefully-hoist: https://pnpm.io/package_json#pnpmpackageextensions

I wonder why vite doesn't find the dependency though as it should resolve xyz from location of svelte-abc, which does work with pnpm.

[bluwy]

I'm not really sure how pnpm.packageExtensions would help here though, Vite's prebundling needs the forced optimizeDeps.include-ed xyz dependency to be in <cwd>/node_modules. The prebundling code has esbuild's absWorkindDir set to process.cwd() (Source). So esbuild would try to resolve xyz at <cwd>/node_modules/xyz.

Also regarding this comment I missed out:

if vite really doesn't consider dependencies of excluded dependencies for optimization that could be a bug.

I'm not sure if this is actually a bug, since the idea of prebundling is node_modules/xyz => node_modules/.vite/xyz.js. If we exclude xyz, then Vite doesn't care about it's nested dependencies. I don't think Vite's on-the-fly pre-bundling handles this too, it would hit the same hoisting issue.

[benmccann]

I sent a PR to make this change in SvelteKit: sveltejs/kit#2137

[dominikg]

@benmccann why not here? The problem isn't specific to sveltekit and doing it here would benefit all users of vite-plugin-svelte.

Removing the entries: [] thing in kit is likely still needed, but automatic handling of optimizeDeps.exclude (and ssr.noExternal) should be in vite-plugin-svelte imho

[benmccann]

Yeah, it probably should live here. Though in that case we should also move ssr.noExternal as well.

The one nice thing about having it live in SvelteKit is that I can more easily play around with it. If it lives here I would need to get any changes here and then update the package in SvelteKit. That's not too big of a deal though

[benbender]

Printing a warning is a good thing, asking users to start pestering library authors/maintainers to use a modern template like sveltekit package or follow it's "distribute as esm, include preprocessed .svelte" pattern

Hey there, as I'm one of the users and lib-authors who are stumbled into this rabbit-hole, I just wanted to note, that it may be of great benefit to have a small set of templates with clearly stated "when to use" and "when not use"-sections which would need to be updated according to updates in this ongoing investigation.

At the moment its just a jungle of outdated knowledge and contradicting advises. Worsened by the fact, that those are being mixed up in the transitions between plain-svelte, sapper & sveltekit...

So I think there is, at least until those issues may be automatically handled, also a problem on the documentation-side of things. F.e. I don't even know, which modern template like sveltekit package is referenced here by @dominikg.

It may also be a good idea to have those packages/templates as they could be referenced in issues and won't be outdated if a user stumbles upon the issue later on if proposed knowledge and/or solutions are changing - as it is today...

[dominikg]

you can use sveltekit to develop and publish svelte libraries: https://kit.svelte.dev/docs#packaging

We have some usage documentation here https://github.com/sveltejs/vite-plugin-svelte/tree/main/packages/vite-plugin-svelte#importing-third-party-svelte-libraries

tbh i'm not sure where the documentation "how to publish svelte libraries the right way" is, so this is indeed something that could be improved (off topic in this repo, maybe ask on svelte discord where this could be and contribute?)

[bluwy]

Once/if vitejs/vite#4634 gets merged, we would also need to optimize dependencies of svelte libraries, e.g.optimizeDeps.include: ["svelte-lib/node_modules/nested-dep"], so if one of the dependencies are exported in cjs only, it'll still work in vite.

[dominikg]

released in https://github.com/sveltejs/vite-plugin-svelte/blob/main/packages/vite-plugin-svelte/CHANGELOG.md#100-next18