-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: ssr.optimizeDeps #8917
feat: ssr.optimizeDeps #8917
Conversation
✅ Deploy Preview for vite-docs-main canceled.
|
One thing that is missing here is to avoid optimizing all deps when
to:
We don't need to optimize ESM dependencies in SSR |
It is linked in the comment, but for completeness here. Another option for the API is accepting a function: optimizeDeps: ssr => ({
...
}) But not every option makes sense in ssr mode, and this will break the whole ecosystem that relies on adding entries to |
What is the difference between:
|
Note: same as just
|
Is esbuild pre-bundling only about CJS -> ESM conversion? I guess it's only needed because Vite only understands ESM? (E.g. to be able to use Is it correct that Vite plugins are applied to VikePress (the docs framework used by |
Yes, adding a dep to |
Ok. This PR seems good to me. The only thing I'm wondering is whether a user config |
I was going to ask the same thing about whether the configuration is necessary. If it's CJS + external then excluding it will result in a breakage. If it's not CJS + external, is there a potential benefit? |
We need this, @cyco130 showed today a case in vavite where he needs to use |
We talked with Evan and Anthony about the PR, and they both think this direction is good. Let's merge it and continue to build on top of it. I'll work on the heuristics change in a future PR. @benmccann I agree with you about |
@patak-dev I was trying this out in a project, and I get the below error
configuration is
any idea what it could be, or any pointers what to check ? |
@jiby-aurum not really without a proper repro. If you could craft a minimal reproduction, please create a new issue. It would be interesting if you try first without using electron, so the repro is easier to investigate. |
@patak-dev I will play a bit more around if I can figure it out, if not will try creating a minimal repro. thanks |
Description
Context:
Build SSR
We were using
@rollup/plugin-commonjs
, needed fornoExternal
CJS deps in Vite v2. Forexternal
deps nothing is needed. In v3 we replace the plugin with esbuild deps optimization. Implemented in:Dev SSR
There is already interop, but it isn't perfect. Allowing users to optimize explicitly defined deps is a way to deal with problematic CJS deps in the SSR dev server. Implemented in:
Issue
In the PRs above we optimized every
noExternal
dep by default (during build we have the complete list, during dev only the explicit ones, but SSR dev interop is normally enough)We also made
optimizeDeps.include/exclude
configure the way SSR optimization works. The problem is that the client env is quite different from the SSR env. @brillout, @benmccann, and others have been pushing hard for deps to beexternal
by default (that we got done in v3).We have conflicting requirements here, we have recommended people to use both
include
andexternal
.For the client, we need to optimize deps because:
For SSR, we want to optimize deps only because:
noExternal
and they are CJSSo for the browser, we want people to add things to
include
, but we don't want all these deps to be optimized during SSR. The best is to leave ESM deps untouched.This PRs allows us to avoid the conflict by introducing a new
ssr.optimizeDeps
experimental option. So users can continue to useconfig.optimizeDeps
for client only. esbuildOptions modifications are also specific to the client or ssr.There are no overrides from
optimizeDeps
tossr.optimizeDeps
, both are independent for the reasons above.Fixes
The PR also takes into account
exclude
to define theexternal
scheme started by @sodatea. Soda, I moved the plugin from config to the optimizer because we have the context needed (dev/build, ssr/non-ssr needed).Refactor
getDepsOptimizer(config, { ssr })
=>getDepsOptimizer(config, ssr)
What is the purpose of this pull request?