fix: Solve issue where worker thread didn't have access to fetch #3012
Conversation
src/compiler/sys/stencil-sys.ts
Outdated
| @@ -505,7 +505,7 @@ export const createSystem = (c?: { logger?: Logger }) => { | |||
| return results; | |||
| }; | |||
|
|
|||
| const fetch = global.fetch; | |||
| const fetch = self?.fetch ?? global?.fetch ?? window?.fetch; | |||
There was a problem hiding this comment.
Truthfully, this can be executed in multiple spaces. So what I'm trying to do here is worker, then node, then window. I'm open to swapping the order here too.
There was a problem hiding this comment.
I couple questions/maybe asks here -
- How do
selfandwindowdiffer here? - Can we add a comment here to explain how/why we chose this order?
- In what circumstances do we use the
fetchonself,global, &window?
There was a problem hiding this comment.
self is the global namespace object used within a web worker.
window is the browser's global namespace object (I reorganized this to check the reference on that second)
global is Node's global namespace object. https://nodejs.org/api/globals.html#globals_global
I did switch these to do worker => browser => node
There was a problem hiding this comment.
👍 I think that's what I'm looking for in the the ask for a comment. ATM, the comment is kind of repeating the code:
// Reference fetch from a worker thread, then a browser thread, then a node thread.
const fetch =
typeof self !== 'undefined'
? self?.fetch
: typeof window !== 'undefined'
? window?.fetch
: typeof global !== 'undefined'
? global?.fetch
: undefined;which doesn't do "future Will and Ryan" much good. Ideally we explain why we choose this order (why is window over global, and self over both of them?) and potentially the context they're used in (what you've got in the previous comment)
| @@ -57,7 +57,6 @@ export class AppRoot { | |||
| } | |||
|
|
|||
| async compile() { | |||
| console.clear(); | |||
There was a problem hiding this comment.
When running this app, the console gets cleared after the worker loads, so the traspile problems weren't immediately obvious when I opened dev tools.
PS: We should figure out how to automate this with Karma.
There was a problem hiding this comment.
Ah, I see now. It was clearing debugging statements in this component itself
And yes! Can you please put an item on the refinement sheet to discuss next week?
| @@ -70,7 +70,7 @@ export const cssTemplatePlugin = { | |||
| async load(id: string) { | |||
| let code = styleImports.get(id.split('?')[0]); | |||
| if (code) { | |||
| const results = await stencil.compile(code, { | |||
| const results = await stencil.transpile(code, { | |||
There was a problem hiding this comment.
compile doesn't exist on the Stencil compiler API.
There was a problem hiding this comment.
Was this being masked by #3012 (comment) clearing the console?
There was a problem hiding this comment.
This failed when running the start command in this directory, because compile didn't exist. It was a TypeScript error that was accurate, I'm assuming because it's been a while since this browser-compile project was started up.
The comment you reference actually covered up errors in the worker, which if it wasn't here, would have immediately told me that how we get the fetch reference for the in-memory system was erroring.
…fetch (#3012) add a more flexible way of assigning `fetch` based on the environment STENCIL-84: Investigate Non-Transpiled Stencil Code in Web API
Closes #3011
The worker thread's in-memory system did not have the correct reference to fetch.
How to test:
npm run build/test/browser-compiledirectory.npm i && npm startOriginally, before this, the transpile results pane displayed empty and there were multiple errors in dev tools.