Move Babel to web-worker #1346
Move Babel to web-worker #1346
Conversation
bvaughn
changed the title
|
Deploy preview ready! Built with commit fde40ac |
bvaughn
force-pushed
the
move-babel-to-web-worker
branch
from
5557da3
to
b10bb47
Compare
|
After commit be69e4a I'm seeing a bit of weirdness with the env plug-in in a web-worker. When I pass it options, it errors with:
But if I try to add the preset without options, it errors with:
|
This isn't quite working yet but I'm going to commit to share with others.
|
@azz moved prettier to a worker on the playground and it worked really well btw :) |
|
Nice! 😄 I'd love to do it for Flow too, after this lands. That's the most painful REPL in terms of locking the UI. |
js/repl/Worker.js
Outdated
| import compile from "./compile"; | ||
| import registerPromiseWorker from "promise-worker/register"; | ||
|
|
||
| declare var Babel: any; |
There was a problem hiding this comment.
Do webpack externals not work in a web worker? require('babel-standalone') should be the same as referencing the global Babel.
There was a problem hiding this comment.
Hm. Babel (or babel-standalone) isn't one of the things we alias though, is it?
There was a problem hiding this comment.
Oh good catch, we don't alias it because it's lazy loaded. Sorry for the confusion @bvaughn :D
js/repl/loadBabel.js
Outdated
| @@ -39,13 +48,12 @@ export default function loadBabel( | |||
| return; | |||
| } | |||
|
|
|||
| let version = config.version; | |||
There was a problem hiding this comment.
I think the earlier position of this code was better, since the comment explains what this variable is for (config.version is passed in dev, which is mentioned in the comment)
There was a problem hiding this comment.
Didn't actually intend to move it. Have just been iterating on this code.
|
Nice work! |
| case "compile": | ||
| return compile(message.code, message.config); | ||
|
|
||
| case "getBabelVersion": |
There was a problem hiding this comment.
Ooh, I should totally steal this idea rather than building a custom file
bvaughn
changed the title
|
Thanks @xtuc! Wrapped up the last thing this morning. This PR should be good to go! I tested Chrome, Firefox, Safari, Edge, Brave on Android, and iOS Safari. PS We have a layout issue on iOS Safari but that's not related to this PR. |
…e for type clarity Compile error must be a string now that we compile in a web-worker. (Errors can't be passed from worker to main.) Technically eval error could be an error object, but since we weren't using the stack anyway, I think consistency is nice for these two properties.
This is no longer needed since we're using the inline worker-loader
|
Thanks all for the feedback and reviews! I made a couple of small tweaks and tidied up. As soon as CI finishes, I'm going to merge this. 😄 |
Resolves #1342
Perf Impact
Here's a super-unscientific example of processing a large UMD file while scrolling around the page.
Before
#### After