-
-
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): support external true #10939
Conversation
const { ssr } = config | ||
if (ssr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed const { ssr } = config
as it's already define above. Removed if (ssr) {
as it's always true.
Co-authored-by: Ben McCann <[email protected]>
👍 Sounds like a good plan. As always, I strongly recommend to always externalize SSR dependencies by default. (Because SSR dependencies often crash when processed by a bundler. For example when they use |
Just a note (mostly to myself): this is a bit different than defaulting everything to external because |
I don't think this is what the PR does. Explicitly defined means |
Description
Support
ssr.external: true
to default all packages to external even if they are linked.I found this handy in tests in monorepos where we want to emulate the usual case of all packages externaled, or when using
ssrLoadModule
to load an arbitrary file and we want to always external packages as we don't care about HMR.Additional context
cc @brillout which brought up something similar some time ago #9367 (reply in thread)
re whether we want to change the default, perhaps we should see how SSR optimize deps help first in Vite 5 as we want to keep Vite 4 less breaking.
Notes on test: I made
__tests__
a workspace package to unit testssrExternal
. Feels a bit weird but this avoids adding tests as e2e 😬What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).