-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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: Add ssr.noExternal = true
option
#4490
Conversation
Maybe we should call this I had been thinking that it could be really nice to rename |
@benmccann You're right naming it |
That was my original reasoning of having |
I've also added a new playground test, |
Neat 👌. From a high-level perspective, the code LGTM. |
Looks good to me 👍🏼 |
Today we discussed with the team that we want to prevent more and more options and would like to reuse |
Let's go with |
Good call on the option usage. I've updated this PR to use |
Could you revert your |
Still changes in |
ssr.bundleAll
optionssr.noExternal = true
option
Description
This PR adds a
ssr.noExternal = true
option which disables all externals during the SSR build process. This is effectively the same as manually adding all dependencies of a package tossr.noExternal
.This is helpful when building for Workers environments like Cloudflare Workers, where
require
is not supported at all.This PR also adds a check to the
resolve
plugin to ensure the user doesn't attempt to import any Node.js built-ins, as those wouldn't be supported in the Workers environment.Additional context
This functionality was discussed in a comment thread here: #4230 (comment)
Another option I considered was to imply this behavior from
ssr.target = 'webworker'
. However, that would be a breaking change, and I think it's better to be explicit in this case (maybe somebody does build a webworker runtime someday that supportsrequire
- who knows?)Tests: I'd love to know the most effective way to test this behavior. Since it's exclusive to Worker environments, and it's only related to builds, it's tough to use the playground tests. I'd also want to test the output of the file, meaning unit tests aren't really the right way to go (unless someone can point me to an example).
UPDATE: Tests have been added. See #4490 (comment)
I also changed the name from
bundle
tobundleAll
.UPDATE 2: Changed to
ssr.noExternal = true
per feedback.What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).