Stop noExternalizing linked dependencies by default

[brillout]

Right now, by default, Vite noExternalizes dependencies that are linked.

Instead, I propose to externalize linked deps by default. The user can revert to the current behavior by adding the dependency to ssr.noExternal.

Because:

• It's unexpected: as a user, I assume that a dependency is treated the same regardless of whether it's linked. I remember the first time I was using Vite, it took me quite a while to understand that linked dependencies are treated differenlty. I was thinking "Vite is doing some funky stuff here". As a user, it's a subpar DX to have Vite behave differently for reasons that are unknown to me.
• It's actually broken: as discussed many times before there are many Node.js libraries that will never work with Vite. We need to externalize them, always. That's why Ben and myself we were pushing for always externalizing dependencies by default. The same applies for linked dependencies.

Note that Vite 3.0.0, 3.0.1, 3.0.2 were actually implementing my proposal, which was reverted in 3.0.3. The interesting thing here is that only SvelteKit's development team complained about that change. This is a signal that not many people will miss the current behavior. With my proposed behavior, the SvelteKit's dev team can revert by adding vite-plugin-svelte to ssr.noExternal while developing SvelteKit (I don't know but that's what I suspect SvelteKit is wanting).

Note that, at the time I'm writing this, 3.0.0, 3.0.1, 3.0.2 is 260k downloads (total Vite downloads: 1.1M downloads). So that's a significant amount of users who didn't even realize the change. I would even say that it's likely that only the SvelteKit dev team would ever have noticed shall we have not reverted things in 3.0.3.

CC @patak-dev @bluwy @benmccann.

[aleclarson]

as discussed many times before there are many Node.js libraries that will never work with Vite

Can you paste some links please?

[brillout]

Mainly #4230 (comment); I believe that's when we started considering externalizing by default.

Also to some degree #4340 (but there are a couple of things I was wrong about in that ticket).

As for npm packages that don't/won't work with Vite: see #4230 (e.g. AWS SDK and Firebase being the most prominent ones I'm aware of).

[benmccann]

I think those npm packages fall more in the category of "don't work" with Vite rather than "won't work" with Vite if my memory is correct

[brillout]

My point is that we pushed for externalization by default as a workaround of libraries that "don't work with Vite (regardless of whether they won't in the future).

Some libraries will never work with Vite, simply because they aren't designed to be bundled. E.g. if they use code that is not statically analyzable, for example:

const peerDependency = isUsingReact ? 'react' : 'vue'
// Vite can't handle such dynamically determined dependencies.
await import(peerDependency)

That's the main reason why externalization by default is so important. (It's also about performance.)

Now, if we noExternalize linked dependencies, then we'll hit problems again when users link dependencies that don't work with Vite.

[benmccann]

I may have missed the discussion and @bluwy or @dominikg might know better than I do, but I don't recall Svelte needing to noExternalize linked dependencies? The thing I do remember being an issue was whether we would put just the package name or a glob as an option value.

[patak-dev]

For reference, @bluwy did this PR to fix a SvelteKit bug report

• fix(ssr): no external symlink package #9296

Since there are .ts files in that dependency, I think the correct fix is to add it to noExternal by the user.
But we still went through with the change because AFAICS, v2 also bundled linked dependencies by default. I'm not sure myself what is the correct default. If v2 was already doing this, we could discuss and propose an intentional change for v4.

[brillout]

I'd say: let's do it in a Vite 3 patch, maybe even the next one (since the change is simple to implement AFAICT). Because: https://vitejs.dev/blog/announcing-vite3.html#build-improvements. We already tell users "externalizing dependencies by default". Also doing that for linked deps aligns well here.

[benmccann]

Ok, this isn't really about supporting SvelteKit, but rather pnpm. Although I'm not sure I really understand how bundling linked packages would be intended to work. I guess it can work for simple packages, but once you start having exports or bundling configs in the linked package it seems like it would break down pretty quickly. I think I agree that it's confusing that linked and non-linked packages behave differently and I'd do away with that. I think I'd just call introducing #9296 a bug and revert it. But I'm not sure if I'm missing other implications that @bluwy might be familiar with.

[bluwy]

One of the main reason linked deps are noExternal by default is for DX in development time. Updating code in a monorepo package would immediately update the server as it's tracked via Vite. This is the same reason linked deps are optimizeDeps.exclude by default. Without this we'd have to constantly restart the server in order see the new changes, which isn't great.

We also can't do this in build as it creates an inconsistency where Vite features work in dev, but doesn't work in build. For example, in dev the users are allowed to use raw TS files, import.meta.glob, etc.

• It's unexpected: as a user, I assume that a dependency is treated the same regardless of whether it's linked. I remember the first time I was using Vite, it took me quite a while to understand that linked dependencies are treated differenlty. I was thinking "Vite is doing some funky stuff here". As a user, it's a subpar DX to have Vite behave differently for reasons that are unknown to me.

I think it should be fine if we document this. I agree that it can be confusing when debugging tooling/config. Ideally this ambiguity should be hidden for most users by default because we want the DX to come automatically without any configuration.

• It's actually broken: as discussed many times before there are many Node.js libraries that will never work with Vite. We need to externalize them, always. That's why Ben and myself we were pushing for always externalizing dependencies by default. The same applies for linked dependencies.

Given the reasons above, I think the pros of DX outweighs this. There are things in Vite like making module.exports work in ssrLoadModule that we haven't got around fixing, so it can be improved in the future.

Note that Vite 3.0.0, 3.0.1, 3.0.2 were actually implementing my proposal, which was reverted in 3.0.3. The interesting thing here is that only SvelteKit's development team complained about that change. This is a signal that not many people will miss the current behavior. With my proposed behavior, the SvelteKit's dev team can revert by adding vite-plugin-svelte to ssr.noExternal while developing SvelteKit (I don't know but that's what I suspect SvelteKit is wanting).

As @patak-dev and @benmccann pointed in another thread, the bug report (sveltejs/kit#5542) in SvelteKit isn't related to SvelteKit only (It should've been a Vite issue). The issue is that a user is exporting raw TypeScript in a monorepo package, and it used to work as it's noExternal by default, which goes through the Vite pipeline. It's not related to anything Svelte-specific. If we go along with this proposal, users would almost always have to manually configure their dep as ssr.noExternal if they're working with any linked dep. Even linked dep from external libraries, it would confusing when HMR does not happen by default.

[brillout]

Note that it's more than just about dynamic imports require(someDynamicallyGeneratedId)/import(someDynamicallyGeneratedId): it's also, for example, about the dependencies of some-library that cannot be resolved when using pnpm. (Or we also noExternalize these dependencies that cannot be resolved, but that then increases the potential for breaking.)

Also, I still don't know why Vite is choking when vite-plugin-ssr's server runtime is noExternalize. So that's also potential for a problem I'm not aware of with noExternalizing libraries.

This technique will break libraries. And I don't think it should be the default.

Also because, as a library author, it's not something I expect Vite to do. (You guys expect that because you are Vite expert users. But I'm speaking on behalf of 99% of library authors who are not Vite experts; they will not expect Vite to do this.)

One way to resolve this discussion is to have a new config ssr.noExternalizeLinkedDependencies (or some other name). But, alas, I can see you not even liking that even though it's only a matter of setting a flag to true for you guys. But I think you are being wrong here. (No offense, obviously :-).)

[brillout]

You guys expect that because you are Vite expert users. But I'm speaking on behalf of 99% of library authors who are not Vite experts; they will not expect Vite to do this.

I'd like to emphasize this: I think it's the crux of the disagreement.

And, again, I do agree that the DX that comes with noExternalizing linked dependencies is great. So how about the following then:

1. By default linked dependencies are externalized (but see point 3. below).
2. The user can set a new config ssrExternalLinkedDependencies: undefined | boolean.
3. If ssrExternalLinkedDependencies is undefined then we console.log() an information telling the user: "You have linked dependencies. Set ssrExternalLinkedDependencies to true to enable HMR for your linked dependencies, or to false to suppress this information. More information at https://vitejs.dev/ssrExternalLinkedDependencies".

This way we educate Vite users about the existence of that technique, without taking them by surprise.

If noExternalizing linked dependencies breaks their app, they'll know what to do. (We can tell users that noExtenalizing linked dependencies doesn't always work at https://vitejs.dev/ssrExternalLinkedDependencies.)

[bluwy]

A new option would be great, but as with Vite's philosophy, we try to reduce the number of config options as much as possible. Even if we go with the new option route, I think it's a bit too specific for something to be configured, and I think an alternative solution is like below:

1. Document how linked deps are noExternal by default at https://vitejs.dev/guide/ssr.html#ssr-externals
2. Show that they can prevent that by specifying the dep at ssr.external
3. Note the special features of noExternal
4. Fix any inconsistencies between noExternal and external. I think ideally we want to keep both working as similarly as possible, except noExternal has "extra features".

[brillout]

I guess we can agree to disagree :-)

[brillout]

Discussion on Discord: https://discord.com/channels/804011606160703521/831456449632534538/1009141562808221837 (private channel).

From Discord:

Maybe we should have a high level config option to toggle this behaviour as @brillout suggested, if frameworks start to set this option for users it will be quite confusing.

I agree and that's why 1) I'd disable the flag by default, and 2) recommend frameworks to not set the flag on behalf of the user.

[benmccann]

We just got bit by this unexpected behavior when upgrading from Vite 3.0.0 to Vite 3.0.8 (sveltejs/kit#5861). It cost hours of debugging to figure it out why our project was broken and would have taken much longer if I weren't already aware of this discussion. I think this is a very problematic feature as it assumes users are not using pnpm and quickly falls apart and breaks projects when dependencies have exports defined or have build steps. I understand that this is how Vite 2 worked and that it can be helpful to users in simple cases, but that does not outweigh the fact that the feature is fundamentally incapable of working in many circumstances causing unexplainable and hard to diagnose project breakages

We've still not been able to fix the issue as the workaround is not working for us, but will continue to investigate

[brillout]

A lot of users are having issues with that.

For example today, a high-tier sponsor is having major issues because its monorepo has a lot of CJS packages that use __dirname. (Note that any package that contains __dirname cannot be ssr.noExternal, because __dirname is inherently broken when bundled.)

I told them to add each CJS package of their monorepo that uses __dirname to ssr.external, but that's a big pain for them as they have a lot of them and it's a moving target.

Right now, by default, Vite noExternalizes dependencies that are linked.
Instead, I propose to externalize linked deps by default

I've grown very confident that this proposal is the right move. But at least supporting ssr.external: true would make the workaround dramatically easier.

@bluwy WDYT?

[bluwy]

ssr.external: true (#10939) is still on the board but we haven't quite discussed it yet.

Personally I'd like to have that PR go through, but I also have doubts whether that's the right API design. For example, there's still the inbalance when dealing with optimizeDeps.exclude and there's no equivalent there.

So in a future team meeting maybe we could also figure the right API for it.

---

I told them to add each CJS package of their monorepo that uses __dirname to ssr.external, but that's a big pain for them as they have a lot of them and it's a moving target.

I guess they could read the package.json and externalized all of the linked ones?

I'm still impartial between this proposal or not. On one hand, Vite today is optimizing the DX for monorepos who embrace ESM, so some sacrifices with CJS is to be expected.

[brillout]

In my experience, the inconsistency of always externalizing except linked deps takes users by surprise — to them it just feels like Vite is broken. Let alone what a "linked dep" means isn't always obvious to users. A lot of users tell me "Vite's behavior is inconsistent".

Always externalizing is simpler for the user to understand.

I agree that noExternalizing linked deps provides a really neat DX for monorepos, and that feature should definitely stay. I just think it should be opt-in instead of the default.

(On a tangent: I think a ssr.noExternal ESM/CJS compat heuristic can be communicated to users in a clear way by 1. telling users "Vite automatically detects, transpiles, and fixes packages that have invalid CJS/ESM code" and 2. a debug flag vite:external-heuristic that tells users why a package is noExternal. I think users will understand this, so that Vite feels clean to users.)

so some sacrifices with CJS is to be expected

Makes sense. Although dynamic imports in ESM also cannot be noExternalized (but that's a much more seldom situation).

I'm not sure how often noExternalize can break ESM code. Probably less often than CJS.

I guess they could read the package.json and externalized all of the linked ones?

👍 I'll propose them that. Although a common issue with monorepos is that package.json is often missing dependencies (because of node_modules/ hoisting).

---

In general, CJS/ESM issues are a huge pain for users. Anything that hleps is a big relief for users. Beyond Rolldown, do you know if there is any plan to improve Vite regarding CJS/ESM issues?

[bluwy]

I'll also put this out there that I rarely hear about the auto noExternal being confusing these days 😅 Not saying it isn't an issue, but I found users to understand the issue once I link them to the docs.

(On a tangent: I think a ssr.noExternal ESM/CJS compat heuristic can be communicated to users in a clear way

Vite doesn't handle CJS code with noExternal today, I think that would error out or at least things like HMR will not work? It also doesn't fix broken ESM/CJS code, especially after #14521. noExternal will only work correctly with ESM.

Beyond Rolldown, do you know if there is any plan to improve Vite regarding CJS/ESM issues?

The linked PR above was my attempt to fix the general CJS/ESM issue in SSR. Essentially leaving importing externalized modules to the runtime directly. This way they will see the import issue early on in dev, and if the runtime (e.g. Bun) has some sort of "fixing" going on, it'll also apply for them.

[stefanholzapfel]

Hi!

I'm late to the party but I have a question.

I do use and really enjoy the feature of linked dependencies being "internalized".

I develop an app that uses a ui component library which resides in a separate repo and it's very efficient to "npm link" it locally and be able to make changes to the ui library that reflect immediately. I also think it's ok to have that as default behavior, since imho in most cases a package is linked locally, that is the reasoning.

BUT my problem is the other way round: When I now try to build the app in a different environment (e.g. a cd pipeline) where the package is loaded from npm and it's not linked, the build breaks because the code is externalized and thus not transpiled. My imports in the app now load the original Typescript code from the dependency.

Is there an option to tell Vite to always noExternalize/internalize the package, no matter how it is provided?

Thanks!

[stefanholzapfel]

Just recognized that this discussion is about SSR, but isn't the different handling of linked dependencies also done/relevant when not using ssr?