Expose JetStream global to workloads#139
Conversation
✅ Deploy Preview for webkit-jetstream-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
danleh
left a comment
danleh
left a comment
There was a problem hiding this comment.
LGTM with nits, but let's wait for Keith.
| }, | ||
| async: true, | ||
| deterministicRandom: true, | ||
| exposeBrowserTest: true, |
There was a problem hiding this comment.
Note to self / Wasm folks: once Transformers.js lands (WIP), we should get rid of this workload (so there is no need to fix this global access).
|
|
||
| if (!!this.plan.deterministicRandom) | ||
| scripts.addDeterministicRandom() | ||
| if (!!this.plan.exposeBrowserTest) { |
There was a problem hiding this comment.
Is my understanding correct that this is a temporary thing, to remove once we have fixed the workloads to not rely on global isD8 or isInBrowser? If yes, let's add a // TODO: Remove this function and exposeBrowserTest once workloads are fixed. to the top of addBrowserTest.
There was a problem hiding this comment.
all the worker tests will keep on using this, since we don't have uniform Worker support in the shells yet.
There was a problem hiding this comment.
Ah, right. Not temporary then, thanks. How hard would it be to polyfill worker stuff for shells?
There was a problem hiding this comment.
Maybe we should push for basic Worker APIs in the shells too, that's what we ultimately did for v8.
Definitely something that's up on the list.
2 participants
Using global variables makes it hard to follow where they originate.
Exposing all the external settings via a global JetStream object makes this a bit more obvious.