-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Expose @babel/eslint-parser/experimental-worker
#13398
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
Expose @babel/eslint-parser/experimental-worker
#13398
Conversation
| MAYBE_PARSE_SYNC: "MAYBE_PARSE_SYNC", | ||
| }; | ||
|
|
||
| class Client { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reorganized this file so that it can both run Babel using worker_threads or in the same thread. The Client class is the "public interface", while the WorkerClient and LocalClient provide the communication implementation.
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit ed21d0b:
|
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/47762/ |
|
CI is failing because of #13399 |
08f6705 to
71d4ff5
Compare
|
The CI error seems related. |
928f6bd to
b6a3f83
Compare
| // fallback to the second entry (the CJS file) | ||
| // In Babel 8 we can simplify this. | ||
| helperSubExports[`./${helperPath}`] = [ | ||
| helperSubExports[`./${path.posix.join("helpers", helperName)}`] = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise it was using mixed / and \.
| ? it.skip | ||
| : it; | ||
|
|
||
| babel7node12("experimental worker works with babel.config.mjs", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice simple test!
5bd5dce to
6d6e7be
Compare
6d6e7be to
ed21d0b
Compare
JLHwung
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with small nits. Awesome work!
| static #worker_threads_cache; | ||
| static get #worker_threads() { | ||
| return (WorkerClient.#worker_threads_cache ??= require("worker_threads")); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: What's the intention of private static getter? Doesn't static #worker_threads = require("worker_threads") work here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is always executed, but require("worker_threads") is only necessary when actually using the experimental implementation.
| static #handleMessage; | ||
|
|
||
| constructor() { | ||
| LocalClient.#handleMessage ??= require("./worker/handle-message.cjs"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto.
Exposing the worker implementation makes it possible for our users to start testing it (and to start using mjs config files) before that we make it the default.
It was initially implemented in #13199 behind the
BABEL_8_BREAKINGflag. This PR reorganizes the implementation so that it can live side-by-side with the sync one.