From c45da3d930a8c284f9bd44abd61d5790daa3cbdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A1ng=20J=C3=B9nli=C3=A0ng?= Date: Tue, 16 Aug 2022 06:43:51 -0400 Subject: [PATCH] Fix a race condition in `@babel/core` (#14843) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Nicolò Ribaudo --- Gulpfile.mjs | 2 +- babel-worker.cjs | 4 +- .../src/config/config-descriptors.ts | 46 +++---- .../src/gensync-utils/functional.ts | 31 +++++ packages/babel-core/test/config-loading.js | 129 +++++++++++------- 5 files changed, 127 insertions(+), 85 deletions(-) create mode 100644 packages/babel-core/src/gensync-utils/functional.ts diff --git a/Gulpfile.mjs b/Gulpfile.mjs index ee3b18906f03..717d0cd4683a 100644 --- a/Gulpfile.mjs +++ b/Gulpfile.mjs @@ -265,7 +265,7 @@ async function buildBabel(useWorker, ignore = []) { const dest = "./" + mapSrcToLib(file.slice(2)); promises.push(worker.transform(file, dest)); } - return Promise.all(promises).finally(() => { + return Promise.allSettled(promises).finally(() => { if (worker.end !== undefined) { worker.end(); } diff --git a/babel-worker.cjs b/babel-worker.cjs index 345ffa11059a..c8d667063ea8 100644 --- a/babel-worker.cjs +++ b/babel-worker.cjs @@ -1,4 +1,4 @@ -const { transformSync } = require("@babel/core"); +const { transformAsync } = require("@babel/core"); const { mkdirSync, statSync, readFileSync, writeFileSync } = require("fs"); const { dirname } = require("path"); const { log } = require("./scripts/utils/logger.cjs"); @@ -32,7 +32,7 @@ exports.transform = async function transform(src, dest) { } log(`Compiling '${chalk.cyan(src)}'...`); const content = readFileSync(src, { encoding: "utf8" }); - const { code } = transformSync(content, { + const { code } = await transformAsync(content, { filename: src, caller: { // We have wrapped packages/babel-core/src/config/files/configuration.js with feature detection diff --git a/packages/babel-core/src/config/config-descriptors.ts b/packages/babel-core/src/config/config-descriptors.ts index fe033eccc4aa..7483516806a2 100644 --- a/packages/babel-core/src/config/config-descriptors.ts +++ b/packages/babel-core/src/config/config-descriptors.ts @@ -1,6 +1,5 @@ -import gensync from "gensync"; - -import type { Handler } from "gensync"; +import gensync, { type Handler } from "gensync"; +import { once } from "../gensync-utils/functional"; import { loadPlugin, loadPreset } from "./files"; @@ -123,35 +122,22 @@ export function createUncachedDescriptors( options: ValidatedOptions, alias: string, ): OptionsAndDescriptors { - // The returned result here is cached to represent a config object in - // memory, so we build and memoize the descriptors to ensure the same - // values are returned consistently. - let plugins: UnloadedDescriptor[]; - let presets: UnloadedDescriptor[]; - return { options: optionsWithResolvedBrowserslistConfigFile(options, dirname), - *plugins() { - if (!plugins) { - plugins = yield* createPluginDescriptors( - options.plugins || [], - dirname, - alias, - ); - } - return plugins; - }, - *presets() { - if (!presets) { - presets = yield* createPresetDescriptors( - options.presets || [], - dirname, - alias, - !!options.passPerPreset, - ); - } - return presets; - }, + // The returned result here is cached to represent a config object in + // memory, so we build and memoize the descriptors to ensure the same + // values are returned consistently. + plugins: once(() => + createPluginDescriptors(options.plugins || [], dirname, alias), + ), + presets: once(() => + createPresetDescriptors( + options.presets || [], + dirname, + alias, + !!options.passPerPreset, + ), + ), }; } diff --git a/packages/babel-core/src/gensync-utils/functional.ts b/packages/babel-core/src/gensync-utils/functional.ts new file mode 100644 index 000000000000..38048ff8d902 --- /dev/null +++ b/packages/babel-core/src/gensync-utils/functional.ts @@ -0,0 +1,31 @@ +import type { Handler } from "gensync"; + +import { isAsync, waitFor } from "./async"; + +export function once(fn: () => Handler): () => Handler { + let result: R; + let resultP: Promise; + return function* () { + if (result) return result; + if (!(yield* isAsync())) return (result = yield* fn()); + if (resultP) return yield* waitFor(resultP); + + let resolve: (result: R) => void, reject: (error: unknown) => void; + resultP = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + + try { + result = yield* fn(); + // Avoid keeping the promise around + // now that we have the result. + resultP = null; + resolve(result); + return result; + } catch (error) { + reject(error); + throw error; + } + }; +} diff --git a/packages/babel-core/test/config-loading.js b/packages/babel-core/test/config-loading.js index 4bbab5973185..089fe599b79e 100644 --- a/packages/babel-core/test/config-loading.js +++ b/packages/babel-core/test/config-loading.js @@ -1,15 +1,15 @@ -import _loadConfigRunner, { - loadPartialConfig, - createConfigItem, -} from "../lib/config/index.js"; +import { + loadOptionsSync, + loadOptionsAsync, + loadPartialConfigSync, + createConfigItemSync, +} from "../lib/index.js"; import path from "path"; import { fileURLToPath } from "url"; import { createRequire } from "module"; const require = createRequire(import.meta.url); -const loadConfig = (_loadConfigRunner.default || _loadConfigRunner).sync; - describe("@babel/core config loading", () => { const FILEPATH = path.join( path.dirname(fileURLToPath(import.meta.url)), @@ -45,7 +45,7 @@ describe("@babel/core config loading", () => { }; } - describe("createConfigItem", () => { + describe("createConfigItemSync", () => { // Windows uses different file paths const noWin = process.platform === "win32" ? it.skip : it; @@ -54,7 +54,7 @@ describe("@babel/core config loading", () => { return {}; } - expect(createConfigItem(myPlugin)).toEqual({ + expect(createConfigItemSync(myPlugin)).toEqual({ dirname: process.cwd(), file: undefined, name: undefined, @@ -69,7 +69,7 @@ describe("@babel/core config loading", () => { } expect( - createConfigItem(myPlugin, { dirname: "/foo", type: "plugin" }), + createConfigItemSync(myPlugin, { dirname: "/foo", type: "plugin" }), ).toEqual({ dirname: "/foo", file: undefined, @@ -80,13 +80,13 @@ describe("@babel/core config loading", () => { }); }); - describe("loadPartialConfig", () => { + describe("loadPartialConfigSync", () => { it("should preserve disabled plugins in the partial config", () => { const plugin = function () { return {}; }; - const opts = loadPartialConfig({ + const opts = loadPartialConfigSync({ ...makeOpts(true), babelrc: false, configFile: false, @@ -105,7 +105,7 @@ describe("@babel/core config loading", () => { return {}; }; - const opts = loadPartialConfig({ + const opts = loadPartialConfigSync({ ...makeOpts(true), babelrc: false, configFile: false, @@ -128,7 +128,7 @@ describe("@babel/core config loading", () => { "nested", ); - const { options } = await loadPartialConfig({ + const { options } = await loadPartialConfigSync({ cwd, filename: path.join(cwd, "file.js"), rootMode: "upward", @@ -139,11 +139,11 @@ describe("@babel/core config loading", () => { }); }); - describe("config file", () => { + describe("loadOptionsSync", () => { it("should load and cache the config with plugins and presets", () => { const opts = makeOpts(); - const options1 = loadConfig(opts).options; + const options1 = loadOptionsSync(opts); expect(options1.plugins.map(p => p.key)).toEqual([ "plugin1", "plugin2", @@ -153,7 +153,7 @@ describe("@babel/core config loading", () => { "plugin3", ]); - const options2 = loadConfig(opts).options; + const options2 = loadOptionsSync(opts); expect(options2.plugins.length).toBe(options1.plugins.length); for (let i = 0; i < options2.plugins.length; i++) { @@ -162,7 +162,7 @@ describe("@babel/core config loading", () => { }); it("should load and cache the config for unique opts objects", () => { - const options1 = loadConfig(makeOpts(true)).options; + const options1 = loadOptionsSync(makeOpts(true)); expect(options1.plugins.map(p => p.key)).toEqual([ "plugin1", "plugin2", @@ -170,7 +170,7 @@ describe("@babel/core config loading", () => { "plugin3", ]); - const options2 = loadConfig(makeOpts(true)).options; + const options2 = loadOptionsSync(makeOpts(true)); expect(options2.plugins.length).toBe(options1.plugins.length); for (let i = 0; i < options2.plugins.length; i++) { @@ -181,11 +181,11 @@ describe("@babel/core config loading", () => { it("should invalidate config file plugins", () => { const opts = makeOpts(); - const options1 = loadConfig(opts).options; + const options1 = loadOptionsSync(opts); process.env.INVALIDATE_PLUGIN1 = true; - const options2 = loadConfig(opts).options; + const options2 = loadOptionsSync(opts); expect(options2.plugins.length).toBe(options1.plugins.length); expect(options2.plugins[0]).not.toBe(options1.plugins[0]); @@ -195,7 +195,7 @@ describe("@babel/core config loading", () => { process.env.INVALIDATE_PLUGIN3 = true; - const options3 = loadConfig(opts).options; + const options3 = loadOptionsSync(opts); expect(options3.plugins.length).toBe(options1.plugins.length); expect(options3.plugins.length).toBe(6); expect(options3.plugins[0]).not.toBe(options1.plugins[0]); @@ -208,11 +208,11 @@ describe("@babel/core config loading", () => { it("should invalidate config file presets and their children", () => { const opts = makeOpts(); - const options1 = loadConfig(opts).options; + const options1 = loadOptionsSync(opts); process.env.INVALIDATE_PRESET1 = true; - const options2 = loadConfig(opts).options; + const options2 = loadOptionsSync(opts); expect(options2.plugins.length).toBe(options1.plugins.length); expect(options2.plugins.length).toBe(6); expect(options2.plugins[5]).not.toBe(options1.plugins[5]); @@ -222,7 +222,7 @@ describe("@babel/core config loading", () => { process.env.INVALIDATE_PRESET2 = true; - const options3 = loadConfig(opts).options; + const options3 = loadOptionsSync(opts); expect(options3.plugins.length).toBe(options1.plugins.length); expect(options3.plugins.length).toBe(6); expect(options3.plugins[4]).not.toBe(options1.plugins[4]); @@ -235,11 +235,11 @@ describe("@babel/core config loading", () => { it("should invalidate the config file and its plugins/presets", () => { const opts = makeOpts(); - const options1 = loadConfig(opts).options; + const options1 = loadOptionsSync(opts); process.env.INVALIDATE_BABELRC = true; - const options2 = loadConfig(opts).options; + const options2 = loadOptionsSync(opts); expect(options2.plugins.length).toBe(options1.plugins.length); expect(options2.plugins.length).toBe(6); expect(options2.plugins[0]).not.toBe(options1.plugins[0]); @@ -252,13 +252,38 @@ describe("@babel/core config loading", () => { }); }); + describe("loadOptionsAsync", () => { + it("should load and cache the config with plugins and presets when executed in parallel", async () => { + const opts = makeOpts(); + + const [options1, options2] = await Promise.all([ + loadOptionsAsync(opts), + loadOptionsAsync(opts), + ]); + expect(options1.plugins.map(p => p.key)).toEqual([ + "plugin1", + "plugin2", + "plugin6", + "plugin5", + "plugin4", + "plugin3", + ]); + + expect(options2.plugins.length).toBe(options1.plugins.length); + + for (let i = 0; i < options2.plugins.length; i++) { + expect(options2.plugins[i]).toBe(options1.plugins[i]); + } + }); + }); + describe("programmatic plugins/presets", () => { it("should not invalidate the plugins when given a fresh object", () => { const opts = makeOpts(); - const options1 = loadConfig(opts).options; + const options1 = loadOptionsSync(opts); - const options2 = loadConfig(Object.assign({}, opts)).options; + const options2 = loadOptionsSync(Object.assign({}, opts)); expect(options2.plugins.length).toBe(options1.plugins.length); for (let i = 0; i < options2.plugins.length; i++) { @@ -269,12 +294,12 @@ describe("@babel/core config loading", () => { it("should not invalidate the plugins when given a fresh arrays", () => { const opts = makeOpts(); - const options1 = loadConfig(opts).options; + const options1 = loadOptionsSync(opts); - const options2 = loadConfig({ + const options2 = loadOptionsSync({ ...opts, plugins: opts.plugins.slice(), - }).options; + }); expect(options2.plugins.length).toBe(options1.plugins.length); for (let i = 0; i < options2.plugins.length; i++) { @@ -285,12 +310,12 @@ describe("@babel/core config loading", () => { it("should not invalidate the presets when given a fresh arrays", () => { const opts = makeOpts(); - const options1 = loadConfig(opts).options; + const options1 = loadOptionsSync(opts); - const options2 = loadConfig({ + const options2 = loadOptionsSync({ ...opts, presets: opts.presets.slice(), - }).options; + }); expect(options2.plugins.length).toBe(options1.plugins.length); for (let i = 0; i < options2.plugins.length; i++) { @@ -301,12 +326,12 @@ describe("@babel/core config loading", () => { it("should invalidate the plugins when given a fresh options", () => { const opts = makeOpts(); - const options1 = loadConfig(opts).options; + const options1 = loadOptionsSync(opts); - const options2 = loadConfig({ + const options2 = loadOptionsSync({ ...opts, plugins: opts.plugins.map(([plg, opt]) => [plg, { ...opt }]), - }).options; + }); expect(options2.plugins.length).toBe(options1.plugins.length); expect(options2.plugins.length).toBe(6); @@ -322,12 +347,12 @@ describe("@babel/core config loading", () => { it("should invalidate the presets when given a fresh options", () => { const opts = makeOpts(); - const options1 = loadConfig(opts).options; + const options1 = loadOptionsSync(opts); - const options2 = loadConfig({ + const options2 = loadOptionsSync({ ...opts, presets: opts.presets.map(([plg, opt]) => [plg, { ...opt }]), - }).options; + }); expect(options2.plugins.length).toBe(options1.plugins.length); expect(options2.plugins.length).toBe(6); @@ -343,11 +368,11 @@ describe("@babel/core config loading", () => { it("should invalidate the programmatic plugins", () => { const opts = makeOpts(); - const options1 = loadConfig(opts).options; + const options1 = loadOptionsSync(opts); process.env.INVALIDATE_PLUGIN6 = true; - const options2 = loadConfig(opts).options; + const options2 = loadOptionsSync(opts); expect(options2.plugins.length).toBe(options1.plugins.length); expect(options2.plugins.length).toBe(6); @@ -363,11 +388,11 @@ describe("@babel/core config loading", () => { it("should invalidate the programmatic presets and their children", () => { const opts = makeOpts(); - const options1 = loadConfig(opts).options; + const options1 = loadOptionsSync(opts); process.env.INVALIDATE_PRESET3 = true; - const options2 = loadConfig(opts).options; + const options2 = loadOptionsSync(opts); expect(options2.plugins.length).toBe(options1.plugins.length); expect(options2.plugins.length).toBe(6); @@ -390,7 +415,7 @@ describe("@babel/core config loading", () => { plugins: [fooPlugin], }; - expect(() => loadConfig(opts)).toThrow( + expect(() => loadOptionsSync(opts)).toThrow( /\.inherits must be a function, or undefined/, ); }); @@ -407,7 +432,7 @@ describe("@babel/core config loading", () => { plugins: [fooPlugin], }; - expect(() => loadConfig(opts)).toThrow( + expect(() => loadOptionsSync(opts)).toThrow( /\.visitor cannot contain catch-all "enter" or "exit" handlers\. Please target individual nodes\./, ); }); @@ -415,29 +440,29 @@ describe("@babel/core config loading", () => { describe("caller metadata", () => { it("should pass caller data through", () => { - const options1 = loadConfig({ + const options1 = loadOptionsSync({ ...makeOpts(), caller: { name: "babel-test", someFlag: true, }, - }).options; + }); expect(options1.caller.name).toBe("babel-test"); expect(options1.caller.someFlag).toBe(true); }); it("should pass unknown caller data through", () => { - const options1 = loadConfig({ + const options1 = loadOptionsSync({ ...makeOpts(), caller: undefined, - }).options; + }); expect(options1.caller).toBeUndefined(); }); it("should pass caller data to test functions", () => { - const options1 = loadConfig({ + const options1 = loadOptionsSync({ ...makeOpts(), caller: { name: "babel-test", @@ -453,7 +478,7 @@ describe("@babel/core config loading", () => { ast: false, }, ], - }).options; + }); expect(options1.comments).toBe(false); expect(options1.ast).not.toBe(false);