From 407481806738b60de636f479fd527a17a8872f5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Sun, 1 Mar 2020 19:46:58 +0100 Subject: [PATCH 1/4] Pass URLs to import() rather than paths --- packages/babel-core/src/config/files/module-types.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/babel-core/src/config/files/module-types.js b/packages/babel-core/src/config/files/module-types.js index 2cffeb613d84..4c761597f9f1 100644 --- a/packages/babel-core/src/config/files/module-types.js +++ b/packages/babel-core/src/config/files/module-types.js @@ -1,6 +1,7 @@ import { isAsync, waitFor } from "../../gensync-utils/async"; import type { Handler } from "gensync"; import path from "path"; +import { pathToFileURL } from "url"; let import_; try { @@ -55,6 +56,8 @@ async function loadMjsDefault(filepath: string) { ); } - const module = await import_(filepath); + // import() expects URLs, not file paths. + // https://github.com/nodejs/node/issues/31710 + const module = await import_(pathToFileURL(filepath)); return module.default; } From 2204c30139e3ad4656206c18c81f54ae130c1401 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Sun, 1 Mar 2020 22:16:01 +0100 Subject: [PATCH 2/4] Make tests pass --- babel.config.js | 30 ++++++++++++++++++++---- eslint/babel-eslint-parser/test/index.js | 3 ++- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/babel.config.js b/babel.config.js index 1ec169604ab5..b1116f20c2bb 100644 --- a/babel.config.js +++ b/babel.config.js @@ -12,6 +12,8 @@ module.exports = function(api) { }; const envOpts = Object.assign({}, envOptsNoTargets); + const compileDynamicImport = env === "test" || env === "development"; + let convertESM = true; let ignoreLib = true; let includeRegeneratorRuntime = false; @@ -106,11 +108,10 @@ module.exports = function(api) { ["@babel/plugin-proposal-optional-chaining", { loose: true }], ["@babel/plugin-proposal-nullish-coalescing-operator", { loose: true }], + compileDynamicImport ? dynamicImportUrlToPath : null, + compileDynamicImport ? "@babel/plugin-proposal-dynamic-import" : null, + convertESM ? "@babel/transform-modules-commonjs" : null, - // Until Jest supports native mjs, we must simulate it 🤷 - env === "test" || env === "development" - ? "@babel/plugin-proposal-dynamic-import" - : null, ].filter(Boolean), overrides: [ { @@ -152,3 +153,24 @@ module.exports = function(api) { return config; }; + +// 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. +// NOTE: This plugin must run before @babel/plugin-transform-modules-commonjs +function dynamicImportUrlToPath({ template }) { + return { + visitor: { + CallExpression(path) { + if (path.get("callee").isImport()) { + path.get("arguments.0").replaceWith( + template.expression.ast` + require("url").fileURLToPath(${path.node.arguments[0]}) + ` + ); + } + }, + }, + }; +} diff --git a/eslint/babel-eslint-parser/test/index.js b/eslint/babel-eslint-parser/test/index.js index 1c14ec3f4f0a..8b5266d2876d 100644 --- a/eslint/babel-eslint-parser/test/index.js +++ b/eslint/babel-eslint-parser/test/index.js @@ -1,4 +1,5 @@ import path from "path"; +import { pathToFileURL } from "url"; import escope from "eslint-scope"; import unpad from "dedent"; import { parseForESLint } from "../src"; @@ -71,7 +72,7 @@ describe("Babel and Espree", () => { const espreePath = require.resolve("espree", { paths: [path.dirname(require.resolve("eslint"))], }); - espree = await import(espreePath); + espree = await import(pathToFileURL(espreePath)); }); describe("compatibility", () => { From 6238e5b429e06f123c8d00d63cf3cec3e6237d0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Mon, 2 Mar 2020 12:23:41 +0100 Subject: [PATCH 3/4] =?UTF-8?q?(=E2=95=AF=C2=B0=E2=96=A1=C2=B0=EF=BC=89?= =?UTF-8?q?=E2=95=AF=EF=B8=B5=20=E2=94=BB=E2=94=81=E2=94=BB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- babel.config.js | 8 +++++++- eslint/babel-eslint-parser/test/index.js | 13 ++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/babel.config.js b/babel.config.js index b1116f20c2bb..82b188adf067 100644 --- a/babel.config.js +++ b/babel.config.js @@ -1,5 +1,7 @@ "use strict"; +const currentNodeSupportsURL = !!require("url").pathToFileURL; + module.exports = function(api) { const env = api.env(); @@ -108,7 +110,11 @@ module.exports = function(api) { ["@babel/plugin-proposal-optional-chaining", { loose: true }], ["@babel/plugin-proposal-nullish-coalescing-operator", { loose: true }], - compileDynamicImport ? dynamicImportUrlToPath : null, + // See eslint/babel-eslint-parser/test/index.js#L80 to understand why + // we need the currentNodeSupportsURL check. + compileDynamicImport && currentNodeSupportsURL + ? dynamicImportUrlToPath + : null, compileDynamicImport ? "@babel/plugin-proposal-dynamic-import" : null, convertESM ? "@babel/transform-modules-commonjs" : null, diff --git a/eslint/babel-eslint-parser/test/index.js b/eslint/babel-eslint-parser/test/index.js index 8b5266d2876d..23453881be87 100644 --- a/eslint/babel-eslint-parser/test/index.js +++ b/eslint/babel-eslint-parser/test/index.js @@ -72,7 +72,18 @@ describe("Babel and Espree", () => { const espreePath = require.resolve("espree", { paths: [path.dirname(require.resolve("eslint"))], }); - espree = await import(pathToFileURL(espreePath)); + + // Dynamic import on Node.js needs a file URL, not a path. + // However, pathToFileURL is only supported starting from Node.js 10. + // In older versions, we can pass an absolute path to import() and configure + // babel.config.js so that import() is transpiled to something that works + // with absolute paths. + if (!pathToFileURL) { + // TODO: Remove in Babel 8 + espree = await import(espreePath); + } else { + espree = await import(pathToFileURL(espreePath)); + } }); describe("compatibility", () => { From 21a41a8b7b8dc54026a1c01045899ea27988d6ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Mon, 2 Mar 2020 13:19:51 +0100 Subject: [PATCH 4/4] =?UTF-8?q?(=E2=95=AF=C2=B0=E2=96=A1=C2=B0=EF=BC=89?= =?UTF-8?q?=E2=95=AF=EF=B8=B5=20=E2=94=BB=E2=94=81=E2=94=BB=20=20=20=20=20?= =?UTF-8?q?=20=20=20=20=20=20(=E2=95=AF=C2=B0=E2=96=A1=C2=B0=EF=BC=89?= =?UTF-8?q?=E2=95=AF=EF=B8=B5=20=E2=94=BB=E2=94=81=E2=94=BB=20=20=20=20=20?= =?UTF-8?q?=20=20=20=20=20=20=20=20(=E2=95=AF=C2=B0=E2=96=A1=C2=B0?= =?UTF-8?q?=EF=BC=89=E2=95=AF=EF=B8=B5=20=E2=94=BB=E2=94=81=E2=94=BB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- babel.config.js | 51 +++++++++++++++--------- eslint/babel-eslint-parser/test/index.js | 12 +----- 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/babel.config.js b/babel.config.js index 82b188adf067..d25af01ef093 100644 --- a/babel.config.js +++ b/babel.config.js @@ -1,7 +1,5 @@ "use strict"; -const currentNodeSupportsURL = !!require("url").pathToFileURL; - module.exports = function(api) { const env = api.env(); @@ -110,11 +108,7 @@ module.exports = function(api) { ["@babel/plugin-proposal-optional-chaining", { loose: true }], ["@babel/plugin-proposal-nullish-coalescing-operator", { loose: true }], - // See eslint/babel-eslint-parser/test/index.js#L80 to understand why - // we need the currentNodeSupportsURL check. - compileDynamicImport && currentNodeSupportsURL - ? dynamicImportUrlToPath - : null, + compileDynamicImport ? dynamicImportUrlToPath : null, compileDynamicImport ? "@babel/plugin-proposal-dynamic-import" : null, convertESM ? "@babel/transform-modules-commonjs" : null, @@ -160,23 +154,44 @@ 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. -// NOTE: This plugin must run before @babel/plugin-transform-modules-commonjs +// 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 }) { - return { - visitor: { - CallExpression(path) { - if (path.get("callee").isImport()) { - path.get("arguments.0").replaceWith( - template.expression.ast` + const currentNodeSupportsURL = !!require("url").pathToFileURL; + + 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")); + } + }, + }, + }; + } } diff --git a/eslint/babel-eslint-parser/test/index.js b/eslint/babel-eslint-parser/test/index.js index 23453881be87..16204fbb9a53 100644 --- a/eslint/babel-eslint-parser/test/index.js +++ b/eslint/babel-eslint-parser/test/index.js @@ -73,17 +73,7 @@ describe("Babel and Espree", () => { paths: [path.dirname(require.resolve("eslint"))], }); - // Dynamic import on Node.js needs a file URL, not a path. - // However, pathToFileURL is only supported starting from Node.js 10. - // In older versions, we can pass an absolute path to import() and configure - // babel.config.js so that import() is transpiled to something that works - // with absolute paths. - if (!pathToFileURL) { - // TODO: Remove in Babel 8 - espree = await import(espreePath); - } else { - espree = await import(pathToFileURL(espreePath)); - } + espree = await import(pathToFileURL(espreePath)); }); describe("compatibility", () => {