Skip to content

Commit

Permalink
Don't compile import() in development (#12288)
Browse files Browse the repository at this point in the history
  • Loading branch information
nicolo-ribaudo authored Nov 18, 2020
1 parent 94d1160 commit 6a0e909
Show file tree
Hide file tree
Showing 10 changed files with 127 additions and 173 deletions.
49 changes: 2 additions & 47 deletions babel.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,10 @@ module.exports = function (api) {
const envOptsNoTargets = {
loose: true,
shippedProposals: true,
modules: false,
};
const envOpts = Object.assign({}, envOptsNoTargets);

const compileDynamicImport =
env === "test" || env === "development" || env === "test-legacy";

let convertESM = true;
let ignoreLib = true;
let includeRegeneratorRuntime = false;
Expand Down Expand Up @@ -115,9 +113,8 @@ module.exports = function (api) {
"@babel/proposal-object-rest-spread",
{ useBuiltIns: true, loose: true },
],
compileDynamicImport ? dynamicImportUrlToPath : null,
compileDynamicImport ? "@babel/plugin-proposal-dynamic-import" : null,

convertESM ? "@babel/proposal-export-namespace-from" : null,
convertESM ? "@babel/transform-modules-commonjs" : null,
].filter(Boolean),
overrides: [
Expand Down Expand Up @@ -163,45 +160,3 @@ module.exports = function (api) {

return config;
};

// !!! WARNING !!! Hacks are coming

// import() uses file:// URLs for absolute imports, while require() uses
// file paths.
// Since this isn't handled by @babel/plugin-transform-modules-commonjs,
// we must handle it here.
// However, fileURLToPath is only supported starting from Node.js 10.
// In older versions, we can remove the pathToFileURL call so that it keeps
// the original absolute path.
// NOTE: This plugin must run before @babel/plugin-transform-modules-commonjs,
// and assumes that the target is the current node version.
function dynamicImportUrlToPath({ template, env }) {
const currentNodeSupportsURL =
!!require("url").pathToFileURL && env() !== "test-legacy"; // test-legacy is run on legacy node versions without pathToFileURL support
if (currentNodeSupportsURL) {
return {
visitor: {
CallExpression(path) {
if (path.get("callee").isImport()) {
path.get("arguments.0").replaceWith(
template.expression.ast`
require("url").fileURLToPath(${path.node.arguments[0]})
`
);
}
},
},
};
} else {
// TODO: Remove in Babel 8 (it's not needed when using Node 10)
return {
visitor: {
CallExpression(path) {
if (path.get("callee").isIdentifier({ name: "pathToFileURL" })) {
path.replaceWith(path.get("arguments.0"));
}
},
},
};
}
}
3 changes: 1 addition & 2 deletions eslint/babel-eslint-parser/test/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import path from "path";
import { pathToFileURL } from "url";
import escope from "eslint-scope";
import unpad from "dedent";
import { parseForESLint } from "../src";
Expand Down Expand Up @@ -80,7 +79,7 @@ describe("Babel and Espree", () => {
paths: [path.dirname(require.resolve("eslint"))],
});

espree = await import(pathToFileURL(espreePath));
espree = require(espreePath);
});

describe("compatibility", () => {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"@babel/eslint-parser": "workspace:*",
"@babel/eslint-plugin-development": "workspace:*",
"@babel/eslint-plugin-development-internal": "workspace:*",
"@babel/plugin-proposal-dynamic-import": "^7.10.4",
"@babel/plugin-proposal-export-namespace-from": "^7.12.1",
"@babel/plugin-proposal-object-rest-spread": "^7.11.0",
"@babel/plugin-transform-for-of": "^7.10.4",
"@babel/plugin-transform-modules-commonjs": "^7.10.4",
Expand Down
199 changes: 111 additions & 88 deletions packages/babel-core/test/config-chain.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,28 @@ import util from "util";
import escapeRegExp from "lodash/escapeRegExp";
import * as babel from "../lib";

// "minNodeVersion": "10.0.0" <-- For Ctrl+F when dropping node 10
const supportsESM = parseInt(process.versions.node) >= 12;

const isMJS = file => path.extname(file) === ".mjs";

const skipUnsupportedESM = (esm, name) => {
if (esm && !supportsESM) {
console.warn(
`Skipping "${name}" because native ECMAScript modules are not supported.`,
);
return true;
}
// This can be removed when loadOptionsAsyncInSpawedProcess is removed.
if (esm && process.platform === "win32") {
console.warn(
`Skipping "${name}" because the ESM runner cannot be spawned on Windows.`,
);
return true;
}
return false;
};

// TODO: In Babel 8, we can directly uses fs.promises which is supported by
// node 8+
const pfs =
Expand Down Expand Up @@ -49,15 +71,19 @@ function loadOptions(opts) {
return babel.loadOptions({ cwd: __dirname, ...opts });
}

function loadOptionsAsync(opts) {
return babel.loadOptionsAsync({ cwd: __dirname, ...opts });
function loadOptionsAsync({ filename, cwd = __dirname }, mjs) {
if (mjs) {
// import() crashes with jest
return loadOptionsAsyncInSpawedProcess({ filename, cwd });
}

return babel.loadOptionsAsync({ filename, cwd });
}

// !!!! hack is coming !!!!
// Remove this function when https://github.com/nodejs/node/issues/35889 is resolved.
// Jest supports dynamic import(), but Node.js segfaults when using it in our tests.
async function loadOptionsAsyncInSpawedProcess({ filename, cwd }) {
// !!!! hack is coming !!!!
// todo(Babel 8): remove this section when https://github.com/facebook/jest/issues/9430 is resolved
// We don't check process.versions.node here, it will fail if node does not support esm
// please publish Babel on a modernized node :)
const { stdout, stderr } = await util.promisify(cp.execFile)(
require.resolve("./fixtures/babel-load-options-async.mjs"),
// pass `cwd` as params as `process.cwd()` will normalize `cwd` on macOS
Expand All @@ -67,7 +93,10 @@ async function loadOptionsAsyncInSpawedProcess({ filename, cwd }) {
env: process.env,
},
);
if (stderr) {

const EXPERIMENTAL_WARNING = /\(node:\d+\) ExperimentalWarning: The ESM module loader is experimental\./;

if (stderr.replace(EXPERIMENTAL_WARNING, "").trim()) {
throw new Error(
"error is thrown in babel-load-options-async.mjs: stdout\n" +
stdout +
Expand All @@ -88,10 +117,11 @@ function pairs(items) {
return pairs;
}

async function getTemp(name, fixtureFolder = "config-files-templates") {
async function getTemp(name) {
const cwd = await pfs.mkdtemp(os.tmpdir() + path.sep + name);
const tmp = name => path.join(cwd, name);
const config = name => pfs.copyFile(fixture(fixtureFolder, name), tmp(name));
const config = name =>
pfs.copyFile(fixture("config-files-templates", name), tmp(name));
return { cwd, tmp, config };
}

Expand Down Expand Up @@ -1060,30 +1090,33 @@ describe("buildConfigChain", function () {
);
});

test.each(
[
"babel.config.json",
"babel.config.js",
"babel.config.cjs",
// We can't transpile import() while publishing, and it isn't supported
// by jest.
process.env.IS_PUBLISH ? "" : "babel.config.mjs",
].filter(Boolean),
)("should load %s asynchronously", async name => {
test.each([
"babel.config.json",
"babel.config.js",
"babel.config.cjs",
"babel.config.mjs",
])("should load %s asynchronously", async name => {
const esm = isMJS(name);
if (skipUnsupportedESM(esm, `should load ${name} asynchronously`)) {
return;
}

const { cwd, tmp, config } = await getTemp(
`babel-test-load-config-async-${name}`,
);
const filename = tmp("src.js");

await config(name);

expect(await loadOptionsAsync({ filename, cwd })).toEqual({
...getDefaults(),
filename,
cwd,
root: cwd,
comments: true,
});
await expect(loadOptionsAsync({ filename, cwd }, esm)).resolves.toEqual(
{
...getDefaults(),
filename,
cwd,
root: cwd,
comments: true,
},
);
});

test.each(
Expand All @@ -1094,48 +1127,26 @@ describe("buildConfigChain", function () {
"babel.config.mjs",
]),
)("should throw if both %s and %s are used", async (name1, name2) => {
const { cwd, tmp, config } = await getTemp(
`babel-test-dup-config-${name1}-${name2}`,
);

// We can't transpile import() while publishing, and it isn't supported
// by jest.
const esm = isMJS(name1) || isMJS(name2);
if (
process.env.IS_PUBLISH &&
(name1 === "babel.config.mjs" || name2 === "babel.config.mjs")
skipUnsupportedESM(
esm,
`should throw if both ${name1} and ${name2} are used`,
)
) {
return;
}

const { cwd, tmp, config } = await getTemp(
`babel-test-dup-config-${name1}-${name2}`,
);

await Promise.all([config(name1), config(name2)]);

await expect(
loadOptionsAsync({ filename: tmp("src.js"), cwd }),
loadOptionsAsync({ filename: tmp("src.js"), cwd }, esm),
).rejects.toThrow(/Multiple configuration files found/);
});

if (process.env.IS_PUBLISH) {
test.each(["babel.config.mjs", ".babelrc.mjs"])(
"should load %s asynchronously",
async name => {
const { cwd, tmp, config } = await getTemp(
`babel-test-load-config-async-prepublish-${name}`,
"config-files-templates-prepublish",
);
const filename = tmp("src.js");
await config(name);
expect(
await loadOptionsAsyncInSpawedProcess({ filename, cwd }),
).toEqual({
...getDefaults(),
filename,
cwd,
root: cwd,
comments: true,
});
},
);
}
});

describe("relative", () => {
Expand Down Expand Up @@ -1181,25 +1192,30 @@ describe("buildConfigChain", function () {
".babelrc",
".babelrc.js",
".babelrc.cjs",
// We can't transpile import() while publishing, and it isn't supported
// by jest.
process.env.IS_PUBLISH ? "" : "babel.config.mjs",
".babelrc.mjs",
].filter(Boolean),
)("should load %s asynchronously", async name => {
const esm = isMJS(name);
if (skipUnsupportedESM(esm, `should load ${name} asynchronously`)) {
return;
}

const { cwd, tmp, config } = await getTemp(
`babel-test-load-config-${name}`,
);
const filename = tmp("src.js");

await config(name);

expect(await loadOptionsAsync({ filename, cwd })).toEqual({
...getDefaults(),
filename,
cwd,
root: cwd,
comments: true,
});
await expect(loadOptionsAsync({ filename, cwd }, esm)).resolves.toEqual(
{
...getDefaults(),
filename,
cwd,
root: cwd,
comments: true,
},
);
});

it("should load .babelignore", () => {
Expand All @@ -1220,23 +1236,24 @@ describe("buildConfigChain", function () {
".babelrc.json",
]),
)("should throw if both %s and %s are used", async (name1, name2) => {
const { cwd, tmp, config } = await getTemp(
`babel-test-dup-config-${name1}-${name2}`,
);

// We can't transpile import() while publishing, and it isn't supported
// by jest.
const esm = isMJS(name1) || isMJS(name2);
if (
process.env.IS_PUBLISH &&
(name1 === ".babelrc.mjs" || name2 === ".babelrc.mjs")
skipUnsupportedESM(
esm,
`should throw if both ${name1} and ${name2} are used`,
)
) {
return;
}

const { cwd, tmp, config } = await getTemp(
`babel-test-dup-config-${name1}-${name2}`,
);

await Promise.all([config(name1), config(name2)]);

await expect(
loadOptionsAsync({ filename: tmp("src.js"), cwd }),
loadOptionsAsync({ filename: tmp("src.js"), cwd }, esm),
).rejects.toThrow(/Multiple configuration files found/);
});

Expand All @@ -1260,17 +1277,23 @@ describe("buildConfigChain", function () {
${".babelrc.cjs"} | ${"babelrc-cjs-error"} | ${/Babelrc threw an error/}
${".babelrc.mjs"} | ${"babelrc-mjs-error"} | ${/Babelrc threw an error/}
${"package.json"} | ${"pkg-error"} | ${/Error while parsing JSON - /}
`("should show helpful errors for $config", async ({ dir, error }) => {
const filename = fixture("config-files", dir, "src.js");

// We can't transpile import() while publishing, and it isn't supported
// by jest.
if (process.env.IS_PUBLISH && dir === "babelrc-mjs-error") return;

await expect(
loadOptionsAsync({ filename, cwd: path.dirname(filename) }),
).rejects.toThrow(error);
});
`(
"should show helpful errors for $config",
async ({ config, dir, error }) => {
const esm = isMJS(config);
if (
skipUnsupportedESM(esm, `should show helpful errors for ${config}`)
) {
return;
}

const filename = fixture("config-files", dir, "src.js");

await expect(
loadOptionsAsync({ filename, cwd: path.dirname(filename) }, esm),
).rejects.toThrow(error);
},
);

it("loadPartialConfig should return a list of files that were extended", () => {
const filename = fixture("config-files", "babelrc-extended", "src.js");
Expand Down

This file was deleted.

Loading

0 comments on commit 6a0e909

Please sign in to comment.