fix: apply SSR externalization heuristic to devDependencies

[benmccann]

Description

Use the SSR externalization heuristic for devDependencies just like we do for dependencies. Whether something is in dependencies vs devDependencies doesn't say anything about whether it might be written in CJS or ESM. It's also an undocumented behavior to treat dependencies vs devDependencies differently so is a bit confusing in that regard

Additional context

SvelteKit's helper libraries don't work because they're distributed as ESM (sveltejs/kit#2237). Since Vite assumes they can always be externalized and loaded with require Vite fails to import them

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the Commit Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[Shinigami92]

Sorry for the failing test, seems it's a main branch error from last commit

[patak-dev]

Looks like this was the original intent, as far as I could track it https://github.com/vitejs/vite/blame/b813b00d234ecb500ae688c4da2b11d3c7a3070b/packages/vite/src/node/ssrExternal.ts#L16

I'm a bit confused here. Shouldn't these helpers be in dependencies instead of devDependencies if they are going to be needed during SSR?

[benmccann]

Shouldn't these helpers be in dependencies instead of devDependencies if they are going to be needed during SSR?

Why would that be the case? When I use Rollup, stuff in devDependencies gets bundled, so will generally still be available in production for use in SSR. It's confusing to me that Vite would diverge from the standard behavior and silently set options in a way that I don't have insight into without reading Vite's code.

[brillout]

@patak-js Thanks for the ping. I'm very much interested in that area.

AFAICT the only one who knows the rationale about the externalize heuristic is Evan. I think we should cue him in at some point in the discussion.

I'm going to repeat myself but I really can't see any reason why we don't externalize everything by default. AFAICT there is no need for a heuristic here.

It seems to me that the contributor community thinks that the sole purpose of the externalize heursitic is ESM/CJS compatibility (correct me if I'm wrong; I didn't read some of the Discord discussions).

But, for ESM/CJS handling, I believe (I'm actually pretty confident here) that following will do the trick:

• If the user's package.json has "type": "module" then Vite should keep the user's code ESM
• Otherwise, Vite should transpile the user's code to CJS

That's it. There is no need to transpile dependencies to make ESM/CJS work.

Note that I had 0 users complaining about server dependency problems with vite-plugin-ssr. The reason is because vite-plugin-ssr apps do not use Vite to transpile server code. For example this server entry https://github.com/brillout/vite-plugin-ssr/blob/master/boilerplates/boilerplate-vue/server/index.js is not processed by Vite and is actually a vanilla Node.js app. This means that server dependencies are not processed by Vite either. Thus my users not complaining.

Isomorphic and browser-side dependencies are designed to work with bundlers. Whereas server dependencies are designed to work with Node.js (they actually often don't work with bundlers). That's a fundamental reason why we should externalize everything by default.

So the fundamental underlying question behind this PR is: do we want to externalize everything by default? It would be great to make progress on that decision. From my perspective, this is the biggest change we can make to make SSR rock-solid.

[benmccann]

I largely agree with you @brillout. But that's a bigger change and I don't want to get too off-topic on this or block other improvements to the existing SSR system in the meantime. There's a basic question here, which is should dependencies and devDependencies be treated differently? That's a question whose answer will apply if we implement a longer-term solution as well. I think that by making dependencies and devDependencies work the same as each other instead of differently that we take a small step towards making life better for users

[haoqunjiang]

I agree that SSR externalization needs a major overhaul later.

But this incremental change looks good to me. It's not likely to break existing use cases and solves a real problem.

[brillout]

I don't see the rationale for treating devDepencencies differently than depencencies either, so from my perspective I don't see any problem with this PR.

solves a real problem.

Makes sense — I just approved the PR.

It's nice that we agree to discuss the externalize heursitic from the ground up.

@patak-js How should we proceed? I can open a new GitHub ticket Refactor Externalize Heuristic that sums up all the discussions we had about the externalize heuristic so far.

[brillout]

Btw. one downside of this PR is that it could considerably slow down things. Deps like typescript that usually live in devDependencies may now end up being transpiled by Vite.

[haoqunjiang]

typescript won't be transpiled. Vite will find out it's a CommonJS module and externalize it later, using the same heuristics applied to dependencies.

[patak-dev]

@patak-js How should we proceed? I can open a new GitHub ticket Refactor Externalize Heuristic that sums up all the discussions we had about the externalize heuristic so far.

I think this would be very useful. Evan is going to be looking into SSR soon IIUC so this should help him navigate what was discussed so far. I think it is a good idea to show your proposal but also document fairly other points of view you saw at least with links to the comments/PRs (I think there were some open PRs from Alec for example). I think that it was good to keep SSR experimental so if there are API changes, we are still on time to do them now that the ecosystem is also small enough to coordinate