[babel 8] Move @babel/register transform to a separate worker

[nicolo-ribaudo]

Q	A
Fixed Issues?	Closes #12814
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass?	Yes
Documentation PR Link
Any Dependency Changes?
License	MIT

Moving Babel to a worker allows us to run it asynchronously, which means that:

• it supports .mjs config files
• it lets us convert @babel/core to ESM while still supporting this CJS synchronous require hook
• it makes it easier to avoid transforming plugins/files used by @babel/register with themselves (this check will be unnecessary in the future)

This PR is heavily inspired by #13199: it uses the same strategy to communicate synchronously (with Atomics.wait/Atomics.notify + receiveMessageOnPort), and it has the same architecture with LocalClient/WorkerClient to temporarily (until Babel 8) support both running in the same thread and in a separate thread.

This PR doesn't expose @babel/register/experimental-worker yet because it's .npmignored; I will open a semver-minor PR to expose it after that this one is merged (similarly to how #13199 was then exposed by #13398).

Unfortunately this diff won't be easy to review, so here is a commit-by-commit description:

• Convert @babel/register to CommonJS
  • This package will never work as native ESM, so we should stop developing it as ESM (compiled to CJS).
  • It's ironic that I also had to migrate it from TS to JS, but it's because TS doesn't support (yet?) using .cts files (or .ts files without exports) when also using the --isolatedModules options. These files were almost untyped, so it's not a big loss.
  • GitHub shows packages/babel-register/src/browser.ts as deleted, but it has just been renamed to packages/babel-register/src/browser.js.
• Support running @babel/register asynchronously in a worker
  1. packages/babel-register/src/cache.js was moved to packages/babel-register/src/worker/cache.js without any change. The diff doesn't show the rename because I left a "proxy" CJS package in the old location just in case soneone (for example, our tests) relies on it.
  2. The contents of packages/babel-register/src/node.js have mostly been moved to packages/babel-register/src/hook.js and the non-worker code paths (with all the Module._cache protection logic) should be the same; the differences are that it now calls into LocalClient rather than directly using @babel/core, and compileHook has been renamed to compileBabel7. Some parts of src/node.js have been moved to packages/babel-register/src/worker/transform.js because they need to run in the worker.
• Test against reentrance adds a test for the behavior explained in the src/is-in-register-worker.js comment, but I also run those tests with the non-worker implementation since it doesn't hurt.
• Test both implementations tests the new worker implementation (and the old one). I suggest disabling whitespace diff while reading this commit since all the tests have been indented.
• Test with mjs config file adds a test to check that Babel can run asynchronously.
• Don't expose @babel/register/experimental-worker yet this is just to avoid exposing the new implementation, so that we can have different commits for the Babel 8 support and for the new Babel 7 feature (so that one can go in the changelog, and the other commit can be prefixed with [babel 8] to make it easier to find while writing the Babel 8 changelog). Also, this PR blocks the ESM migration so I don't want to wait for the next minor 😛
• [babel 8] Drawback: plugins must be stored in separate files This is the trade-off when using workers: you cannot pass functions to them. Luckily Babel plugins are not usually defined inline (either they are represented by strings, or they are in a config file), so most of our users won't notice the restriction. For those who do, the solution is to move the plugin to a separate file or to a config file.

[nicolo-ribaudo]

@bmeck I know that you'd like to move ESM loaders to a separate thread so that they can also work for CJS require hooks; you might be interested in this as a real-world use case.

[babel-bot]

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/50224/

[nicolo-ribaudo]

The remaining failures are probably because workers don't flush their console output synchronously 🤔

[nicolo-ribaudo]

To make sure that the console.logs from the worker are printed before the subsequent ones I tried to wait for the stdout to be flushed before waking up the main thread, however stdout is not flushed until the main thread wakes up and yields to the event loop (causing a deadlock).

This is because, by default Node.js first logs messages from the main thread, then yields to the event loop and logs messages from the worker:

Example

In this code the console.log("In worker!") call happens first, however it logs

Received Hi :)
In worker!

const {
  isMainThread,
  parentPort,
  Worker,
  MessageChannel,
  receiveMessageOnPort,
} = require("worker_threads");

if (isMainThread) {
  const worker = new Worker(__filename);

  const mc = new MessageChannel();
  const signal = new Int32Array(new SharedArrayBuffer(4));
  signal[0] = 0;

  worker.postMessage({ port: mc.port1, signal }, [mc.port1]);
  Atomics.wait(signal, 0, 0);

  const { message } = receiveMessageOnPort(mc.port2);
  console.log("Received", message);

  worker.unref();
} else {
  parentPort.on("message", ({ port, signal }) => {
    console.log("In worker!");

    port.postMessage("Hi :)");
    Atomics.store(signal, 0, 1);
    Atomics.notify(signal, 0);
  });
}

The reason for this behavior is that console.log in workers is always scheduled to the main thread asynchronously, while console.log in the main thread is sometimes synchronous (it depends on the OS and on how the node program is executed).

For now I'll accept this behavior and modify our tests not to rely on a specific ordering, and I'll look for a proper fix* if it ever ends up being a real-world problem (I don't think usually plugins console.log things that must absolutely be printed before anything else?).

* The proper fix is to intercept console.* calls, pass them to the main thread when waking it up and logging them immediately.

[Zzzen]

Why not just use return require("worker_threads")?

[nicolo-ribaudo]

Uh right, require caches internally.

[JLHwung]

Suggested change

	{ NODE_OPTIONS: `-r ${registerFile}` },
	{ NODE_OPTIONS: `--require ${registerFile}` },

[JLHwung]

Suggested change

	["-r", registerFile, testFileLog],
	["--require", registerFile, testFileLog],

[bmeck]

@nicolo-ribaudo it would be good to know exactly what moving to a Worker enables. Implementation to be ESM vs CJS is likely not compelling to various Node.js core contributors so any other expectations would be good. I doubt any performance benefits would come from this since it doesn't look like there will be a shared cache so likely it is some other driving force?

[nicolo-ribaudo]

The driving force is that currently some parts of Babel are optionally async (for example config loading, but we have already the infrastructure to allow using async plugin/preset factories and parsers), and even if it's written in CJS Babel already supports ESM plugins which can only be loaded asynchronously.

@babel/register is currently the only exception in the Babel ecosystem where you must make sure that everything you use in Babel can run synchronously; by moving it to a worker we lift that restriction and make it behave like the other Babel packages.

[JLHwung]

This file should be removed.

[JLHwung]

Is this option only needed by packages/babel-register/src/experimental-worker.js? If so can we modify the source so that we don't need to universally apply allowReturnOutsideFunction?

[JLHwung]

Suggested change

	* If @babel/register is imported using the -r/--require flag, we
	* must the worker will have the same flag and we must avoid registering
	* If @babel/register is imported using the -r/--require flag,
	* the worker will have the same flag and we must avoid registering

[nicolo-ribaudo]

I re-reviewed this (usually I don't review my own PRs, but it has been almost a month since I wrote this code), and I'm now merging it with a single ✔️ since it shouldn't affect the Babel 7 behavior anyway.

[overlookmotel]

Sorry I totally missed this PR before the commit landed. @nicolo-ribaudo A few questions:

Performance?

Do you have any figures on how much slower it performs in a worker? (I assume it will be at least somewhat slower due to added overhead of communication back and forth with worker)

Reduce worker communication volume

Could performance be improved by keeping the source map cache (maps var) inside the worker?

• client.transform() would return the transformed code only (not the map).
• map would be stored in memory inside the worker.
• Add client method client.retrieveMap() to get source map object for a file from the worker.
• retrieveSourceMap() callback passed to sourceMapSupport.install() would call client.retrieveMap(filename) to get the map on demand.

i.e. Source map object is only passed from worker to main thread if/when it's needed, rather than upfront for every file which is transformed. Unless the application throws an error, this will not happen at all.

Internal module cache

In Babel 8, you intend to remove the code to maintain a separate module cache for modules required by Babel internally.

While the internal module cache logic can safely be removed from compile.js, I think nodeWrapper.js is still not completely safe to remove.

In main thread, hook.js requires 'source-map-support' package. source-map-support in turn requires a dependency 'from-buffer'. This happens before babel-register is active, so from-buffer is not transpiled, and the untranspiled version will be left in the module cache.

If application code calls require('from-buffer'), Babel's transform will not be executed on the file, which it should be.

Same applies to 'source-map' package, and it's dependency 'whatwg-url'.

[overlookmotel]

Actually, would it be more performant to deal with the code cache in main thread instead of in the worker?

Usually, the majority of files will be unchanged since babel-register was last run, so transformed code can just be retrieved from the cache. If cache was handled in the main thread, this would avoid a large number of round trips to the worker.

If overhead of calling into the worker is significant, I imagine this could avoid much of the penalty.

[nicolo-ribaudo]

Do you have any figures on how much slower it performs in a worker? (I assume it will be at least somewhat slower due to added overhead of communication back and forth with worker)

We investigated this when we moved parsing in @babel/eslint-parser to a worker, and the performance impact has been neutral (#13199 (comment)). I will investigate how moving the cache of @babel/register affects it.

In Babel 8, you intend to remove the code to maintain a separate module cache for modules required by Babel internally.

While the internal module cache logic can safely be removed from compile.js, I think nodeWrapper.js is still not completely safe to remove.

In main thread, hook.js requires 'source-map-support' package. source-map-support in turn requires a dependency 'from-buffer'. This happens before babel-register is active, so from-buffer is not transpiled, and the untranspiled version will be left in the module cache.

We can lazy-require source-map-support, so that it's imported after setting up @babel/register. It feels safer to touch Module/require internals as less as possible!