-
-
Notifications
You must be signed in to change notification settings - Fork 6k
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: webworker ssr target #3151
Conversation
@frandiox an update on this, we discussed it with Evan and he approves this feature. The |
@patak-js That's great news! Right now I'm setting the I hope Thanks for all! |
Description
Reworked #3121 as a feat.
#2996 and #3039 reworked the package resolution to ignore the
browser
field if the plugin is running in ssr mode.See @frandiox comment, if the target is webworker (ie cloudflare workers) then the
browser
field should be respected even in ssr mode.There is a new config option
ssr.target
:We already have a
build.target
but if we reuse it, the users will be forced to modify this option depending on building for the web or usingbuild --ssr
. I think it makes sense to be able to define both in the config file.A
targetWeb
param to the resolve methods instead of the previousssr
one (I think thattargetWeb
is better here thantargetNode
to avoid the need to negate it when using it). In the resolve plugin:#2996 #3039 didn't introduce a regression for Web Workers, because the target was implicitly Node until this point. So they are both fixes for the Node ssr server.
This PR allows ecosystem solutions like vitedge to resolve the browser field as before. There may be other differences that we need to take into account when
ssr.target
iswebworker
, but for @frandiox Vite was working without issues before these changes. I think there is the need for extra configuration that may now be included as default when targeting webworkers. For example theresolve.mainFields
default. I think these could be added in future PRs. As the SSR feature is still labeled experimental,ssr.target
should also be considered experimental.What is the purpose of this pull request?