Add Edge Server Builds for workerd / edge-light #26116
Conversation
facebook-github-bot
added
CLA Signed
React Core Team
sebmarkbage
force-pushed
the
edgebuilds
branch
from
0b8547a
to
1769f4b
Compare
|
We really should be supporting all those environments - including running a Server Components server inside Electron or in React Native on device. Just need someone to own testing and evaluating it. |
sebmarkbage
force-pushed
the
edgebuilds
branch
from
1769f4b
to
9015188
Compare
| "workerd": "./server.edge.js", | ||
| "edge-light": "./server.edge.js", |
There was a problem hiding this comment.
Isn't edge-light supposed to be a "catch-all" for those environments discussed by WinterCG? If yes, why the workerd condition is used here if it points to the same file? What about other conditions like edge-routine, netlify, etc?
There was a problem hiding this comment.
edge-light is specifically Vercel's environment. These are the only ones we've tested and evaluated the other ones if they work as expected. Afaik, the other ones don't have AsyncLocalStorage yet so they'd use the browser build.
There was a problem hiding this comment.
Is there any condition that acts as a catch-all? I understand that it might not work for your use case here but some other libraries might just want to ship something that works in any worker-like environment. The browser condition doesn't work well for that right now.
There was a problem hiding this comment.
Yea, I've looked into that before. Mainly the thing that sucks is that there's no distinction between "serviceworker" and "worker"-like on the server isn't the same a "worker" on the client. E.g. these APIs can't really be used in actual web workers but can be used in "serviceworker". So I've been waiting on further clarification on that before moving further.
I think the notion of these environments just being "a simple worker" is getting to be outdated as more features are added. I suspect it'll be more like a special "wintercg worker" concept or something that's a superset that's useful.
There was a problem hiding this comment.
It's definitely not always possible to use a single condition to catch them all - but as long as you only need some subset of the functionality, then it makes sense to reuse the condition.
Do you have any opinions about Next adding the support for worker condition as well? It just looks so off to me that it resolves browser but not worker
| "./server.node": "./server.node.js", | ||
| "./static": { | ||
| "workerd": "./static.edge.js", | ||
| "edge-light": "./static.edge.js", | ||
| "deno": "./static.browser.js", | ||
| "worker": "./static.browser.js", |
There was a problem hiding this comment.
We currently abuse the browser builds for Web streams derived environments. We already have a special build for Bun but we should also have one for other "edge" runtimes so that we can maximally take advantage of the APIs that exist on each platform.
Wouldn't it make sense to also include the worker condition (that is defined here) when Next.js bundles for the edge-light runtime? The worker condition is IMHO closer to edge-light than the browser condition is to it. So it's a little bit of a weird situation that browser is automatically picked up but the worker is not.
For context - we've shipped the worker condition in Emotion in hope that it would get picked up by tools. There were no strong signals that other conditions would have to be needed and this seemed like a good "catch all" at the time.
What is different in the content of the files that you return here for the edge-light and worker?
There was a problem hiding this comment.
“worker” doesn’t have AsyncLocalStorage. The other two would include require(“node:async_hooks”) which would fail on worker builds.
sebmarkbage
force-pushed
the
edgebuilds
branch
from
9015188
to
ee24df3
Compare
scripts/rollup/bundles.js
Outdated
| @@ -258,6 +258,19 @@ const bundles = [ | |||
| externals: ['react', 'react-dom'], | |||
| }, | |||
|
|
|||
| /******* React DOM Fizz Server Edge *******/ | |||
| { | |||
| bundleTypes: [BUN_DEV, BUN_PROD], | |||
There was a problem hiding this comment.
This isn't right, right? makes more sense to use the NODE ones thought even that needs a refactor
There was a problem hiding this comment.
Yea, copy paste. Those options don't really make sense since they're all shared anyway. We should probably get rid of the bun ones. It's really CJS.
We currently abuse the browser builds for Web streams derived environments. We already have a special build for Bun but we should also have one for [other "edge" runtimes](https://runtime-keys.proposal.wintercg.org/) so that we can maximally take advantage of the APIs that exist on each platform. In practice, we currently check for a global property called `AsyncLocalStorage` in the server browser builds which we shouldn't really do since browsers likely won't ever have it. Additionally, this should probably move to an import which we can't add to actual browser builds where that will be an invalid import. So it has to be a separate build. That's not done yet in this PR but Vercel will follow Cloudflare's lead here. The `deno` key still points to the browser build since there's no AsyncLocalStorage there but it could use this same or a custom build if support is added. DiffTrain build for [01a0c4e](01a0c4e) [View git log for this commit](https://github.com/facebook/react/commits/01a0c4e12c6aa9732d290e13b1452f72d276934d)
Summary: This sync includes the following changes: - **[86c8c8db7](facebook/react@86c8c8db7 )**: test: Don't retry flushActWork if flushUntilNextPaint threw ([#26121](facebook/react#26121)) //<Sebastian Silbermann>// - **[64acd3918](facebook/react@64acd3918 )**: remove unguarded getRootNode call ([#26152](facebook/react#26152)) //<Josh Story>// - **[71cace4d3](facebook/react@71cace4d3 )**: Migrate testRunner from jasmine2 to jest-circus ([#26144](facebook/react#26144)) //<Ming Ye>// - **[c8510227c](facebook/react@c8510227c )**: Treat displayName as undefined ([#26148](facebook/react#26148)) //<Sebastian Markbåge>// - **[55542bc73](facebook/react@55542bc73 )**: Update jest printBasicPrototype config ([#26142](facebook/react#26142)) //<Ming Ye>// - **[6396b6641](facebook/react@6396b6641 )**: Model Float on Hoistables semantics ([#26106](facebook/react#26106)) //<Josh Story>// - **[ef9f6e77b](facebook/react@ef9f6e77b )**: Enable passing Server References from Server to Client ([#26124](facebook/react#26124)) //<Sebastian Markbåge>// - **[35698311d](facebook/react@35698311d )**: Update jest escapeString config ([#26140](facebook/react#26140)) //<Ming Ye>// - **[6ddcbd4f9](facebook/react@6ddcbd4f9 )**: [flow] enable LTI inference mode ([#26104](facebook/react#26104)) //<Jan Kassens>// - **[53b1f69ba](facebook/react@53b1f69ba )**: Implement unstable_getBoundingClientRect in RN Fabric refs ([#26137](facebook/react#26137)) //<Rubén Norte>// - **[594093496](facebook/react@594093496 )**: Update to Jest 29 ([#26088](facebook/react#26088)) //<Ming Ye>// - **[28fcae062](facebook/react@28fcae062 )**: Add support for SVG `transformOrigin` prop ([#26130](facebook/react#26130)) //<Aravind D>// - **[3ff1540e9](facebook/react@3ff1540e9 )**: Prefer JSX in ReactNoop assertions (to combat out-of-memory test runs) ([#26127](facebook/react#26127)) //<Sebastian Silbermann>// - **[01a0c4e12](facebook/react@01a0c4e12 )**: Add Edge Server Builds for workerd / edge-light ([#26116](facebook/react#26116)) //<Sebastian Markbåge>// - **[f0cf832e1](facebook/react@f0cf832e1 )**: Update Flight Fixture to "use client" instead of .client.js ([#26118](facebook/react#26118)) //<Sebastian Markbåge>// - **[03a216070](facebook/react@03a216070 )**: Rename "dom" fork to "dom-node" and "bun" fork to "dom-bun" ([#26117](facebook/react#26117)) //<Sebastian Markbåge>// - **[4bf2113a1](facebook/react@4bf2113a1 )**: Revert "Move the Webpack manifest config to one level deeper ([#26083](facebook/react#26083))" ([#26111](facebook/react#26111)) //<Sebastian Markbåge>// - **[2ef24145e](facebook/react@2ef24145e )**: [flow] upgrade to 0.199.0 ([#26096](facebook/react#26096)) //<Jan Kassens>// - **[922dd7ba5](facebook/react@922dd7ba5 )**: Revert the outer module object to an object ([#26093](facebook/react#26093)) //<Sebastian Markbåge>// - **[9d111ffdf](facebook/react@9d111ffdf )**: Serialize Promises through Flight ([#26086](facebook/react#26086)) //<Sebastian Markbåge>// - **[0ba4698c7](facebook/react@0ba4698c7 )**: Fix async test in React reconciler ([#26087](facebook/react#26087)) //<Ming Ye>// - **[8c234c0de](facebook/react@8c234c0de )**: Move the Webpack manifest config to one level deeper ([#26083](facebook/react#26083)) //<Sebastian Markbåge>// - **[977bccd24](facebook/react@977bccd24 )**: Refactor Flight Encoding ([#26082](facebook/react#26082)) //<Sebastian Markbåge>// - **[d7bb524ad](facebook/react@d7bb524ad )**: [cleanup] Remove unused package jest-mock-scheduler ([#26084](facebook/react#26084)) //<Ming Ye>// - **[6b3083266](facebook/react@6b3083266 )**: Upgrade prettier ([#26081](facebook/react#26081)) //<Jan Kassens>// - **[1f5ce59dd](facebook/react@1f5ce59dd )**: [cleanup] fully roll out warnAboutSpreadingKeyToJSX ([#26080](facebook/react#26080)) //<Jan Kassens>// Changelog: [General][Changed] - React Native sync for revisions 48b687f...fccf3a9 jest_e2e[run_all_tests] Reviewed By: rubennorte Differential Revision: D43305607 fbshipit-source-id: 8da7567ca2a182f4be27788935c2da30a731f83b
Summary: This sync includes the following changes: - **[86c8c8db7](facebook/react@86c8c8db7 )**: test: Don't retry flushActWork if flushUntilNextPaint threw ([facebook#26121](facebook/react#26121)) //<Sebastian Silbermann>// - **[64acd3918](facebook/react@64acd3918 )**: remove unguarded getRootNode call ([facebook#26152](facebook/react#26152)) //<Josh Story>// - **[71cace4d3](facebook/react@71cace4d3 )**: Migrate testRunner from jasmine2 to jest-circus ([facebook#26144](facebook/react#26144)) //<Ming Ye>// - **[c8510227c](facebook/react@c8510227c )**: Treat displayName as undefined ([facebook#26148](facebook/react#26148)) //<Sebastian Markbåge>// - **[55542bc73](facebook/react@55542bc73 )**: Update jest printBasicPrototype config ([facebook#26142](facebook/react#26142)) //<Ming Ye>// - **[6396b6641](facebook/react@6396b6641 )**: Model Float on Hoistables semantics ([facebook#26106](facebook/react#26106)) //<Josh Story>// - **[ef9f6e77b](facebook/react@ef9f6e77b )**: Enable passing Server References from Server to Client ([facebook#26124](facebook/react#26124)) //<Sebastian Markbåge>// - **[35698311d](facebook/react@35698311d )**: Update jest escapeString config ([facebook#26140](facebook/react#26140)) //<Ming Ye>// - **[6ddcbd4f9](facebook/react@6ddcbd4f9 )**: [flow] enable LTI inference mode ([facebook#26104](facebook/react#26104)) //<Jan Kassens>// - **[53b1f69ba](facebook/react@53b1f69ba )**: Implement unstable_getBoundingClientRect in RN Fabric refs ([facebook#26137](facebook/react#26137)) //<Rubén Norte>// - **[594093496](facebook/react@594093496 )**: Update to Jest 29 ([facebook#26088](facebook/react#26088)) //<Ming Ye>// - **[28fcae062](facebook/react@28fcae062 )**: Add support for SVG `transformOrigin` prop ([facebook#26130](facebook/react#26130)) //<Aravind D>// - **[3ff1540e9](facebook/react@3ff1540e9 )**: Prefer JSX in ReactNoop assertions (to combat out-of-memory test runs) ([facebook#26127](facebook/react#26127)) //<Sebastian Silbermann>// - **[01a0c4e12](facebook/react@01a0c4e12 )**: Add Edge Server Builds for workerd / edge-light ([facebook#26116](facebook/react#26116)) //<Sebastian Markbåge>// - **[f0cf832e1](facebook/react@f0cf832e1 )**: Update Flight Fixture to "use client" instead of .client.js ([facebook#26118](facebook/react#26118)) //<Sebastian Markbåge>// - **[03a216070](facebook/react@03a216070 )**: Rename "dom" fork to "dom-node" and "bun" fork to "dom-bun" ([facebook#26117](facebook/react#26117)) //<Sebastian Markbåge>// - **[4bf2113a1](facebook/react@4bf2113a1 )**: Revert "Move the Webpack manifest config to one level deeper ([facebook#26083](facebook/react#26083))" ([facebook#26111](facebook/react#26111)) //<Sebastian Markbåge>// - **[2ef24145e](facebook/react@2ef24145e )**: [flow] upgrade to 0.199.0 ([facebook#26096](facebook/react#26096)) //<Jan Kassens>// - **[922dd7ba5](facebook/react@922dd7ba5 )**: Revert the outer module object to an object ([facebook#26093](facebook/react#26093)) //<Sebastian Markbåge>// - **[9d111ffdf](facebook/react@9d111ffdf )**: Serialize Promises through Flight ([facebook#26086](facebook/react#26086)) //<Sebastian Markbåge>// - **[0ba4698c7](facebook/react@0ba4698c7 )**: Fix async test in React reconciler ([facebook#26087](facebook/react#26087)) //<Ming Ye>// - **[8c234c0de](facebook/react@8c234c0de )**: Move the Webpack manifest config to one level deeper ([facebook#26083](facebook/react#26083)) //<Sebastian Markbåge>// - **[977bccd24](facebook/react@977bccd24 )**: Refactor Flight Encoding ([facebook#26082](facebook/react#26082)) //<Sebastian Markbåge>// - **[d7bb524ad](facebook/react@d7bb524ad )**: [cleanup] Remove unused package jest-mock-scheduler ([facebook#26084](facebook/react#26084)) //<Ming Ye>// - **[6b3083266](facebook/react@6b3083266 )**: Upgrade prettier ([facebook#26081](facebook/react#26081)) //<Jan Kassens>// - **[1f5ce59dd](facebook/react@1f5ce59dd )**: [cleanup] fully roll out warnAboutSpreadingKeyToJSX ([facebook#26080](facebook/react#26080)) //<Jan Kassens>// Changelog: [General][Changed] - React Native sync for revisions 48b687f...fccf3a9 jest_e2e[run_all_tests] Reviewed By: rubennorte Differential Revision: D43305607 fbshipit-source-id: 8da7567ca2a182f4be27788935c2da30a731f83b
We currently abuse the browser builds for Web streams derived environments. We already have a special build for Bun but we should also have one for other "edge" runtimes so that we can maximally take advantage of the APIs that exist on each platform.
In practice, we currently check for a global property called
AsyncLocalStoragein the server browser builds which we shouldn't really do since browsers likely won't ever have it. Additionally, this should probably move to an import which we can't add to actual browser builds where that will be an invalid import. So it has to be a separate build. That's not done yet in this PR but Vercel will follow Cloudflare's lead here.The
denokey still points to the browser build since there's no AsyncLocalStorage there but it could use this same or a custom build if support is added.