From 9eb131b6b59ecd16ab81cf6ac284c636e46b7b3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A1ng=20J=C3=B9nli=C3=A0ng?= Date: Tue, 9 Feb 2021 11:02:50 -0500 Subject: [PATCH 1/8] fix: use semver gte comparison on polyfill version tester --- babel.config.js | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/babel.config.js b/babel.config.js index 1a8b02970890..88f425f0c11a 100644 --- a/babel.config.js +++ b/babel.config.js @@ -183,6 +183,19 @@ function bool(value) { return value && value !== "false" && value !== "0"; } +// A minimum semver GTE implementation +// Limitation: +// - it only supports comparing major and minor version, assuming Node.js will never ship +// features in patch release so we will never need to compare a version with "1.2.3" +// - it assumes `process.versions.node` never contains `,` +// +// @example +// semverGte("8.10", "8.9") // true +// semverGte("8.9", "8.9") // true +// semverGte("9.0", "8.9") // true +// semverGte("8.9", "8.10") // false +const semverGte = `((a,b)=>(([,v,x,w,y]=/(\\d+)\\.(\\d+).*,(\\d+)\\.(\\d+)/.exec([a,b])),+v>+w||v==w&&+x>=+y))`; + // TODO(Babel 8) This polyfills are only needed for Node.js 6 and 8 /** @param {import("@babel/core")} api */ function pluginPolyfillsOldNode({ template, types: t }) { @@ -207,7 +220,7 @@ function pluginPolyfillsOldNode({ template, types: t }) { // require.resolve's paths option has been introduced in Node.js 8.9 // https://nodejs.org/api/modules.html#modules_require_resolve_request_options replacement: template({ syntacticPlaceholders: true })` - parseFloat(process.versions.node) >= 8.9 + ${semverGte}(process.versions.node, "8.9") ? require.resolve : (/* request */ r, { paths: [/* base */ b] }, M = require("module")) => { let /* filename */ f = M._findPath(r, M._nodeModulePaths(b).concat(b)); @@ -239,7 +252,7 @@ function pluginPolyfillsOldNode({ template, types: t }) { // fs.mkdirSync's recursive option has been introduced in Node.js 10.12 // https://nodejs.org/api/fs.html#fs_fs_mkdirsync_path_options replacement: template` - parseFloat(process.versions.node) >= 10.12 + ${semverGte}(process.versions.node, "10.12") ? fs.mkdirSync : require("make-dir").sync `, From cea9f584a42d2ef39e67bfb453a19273e85c6d53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A1ng=20J=C3=B9nli=C3=A0ng?= Date: Tue, 9 Feb 2021 11:30:53 -0500 Subject: [PATCH 2/8] make life easier --- babel.config.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/babel.config.js b/babel.config.js index 88f425f0c11a..50de7fe39868 100644 --- a/babel.config.js +++ b/babel.config.js @@ -194,7 +194,7 @@ function bool(value) { // semverGte("8.9", "8.9") // true // semverGte("9.0", "8.9") // true // semverGte("8.9", "8.10") // false -const semverGte = `((a,b)=>(([,v,x,w,y]=/(\\d+)\\.(\\d+).*,(\\d+)\\.(\\d+)/.exec([a,b])),+v>+w||v==w&&+x>=+y))`; +`((a,b)=>(([,v,x,w,y]=/(\\d+)\\.(\\d+).*,(\\d+)\\.(\\d+)/.exec([a,b])),+v>+w||v==w&&+x>=+y))`; // TODO(Babel 8) This polyfills are only needed for Node.js 6 and 8 /** @param {import("@babel/core")} api */ @@ -220,7 +220,7 @@ function pluginPolyfillsOldNode({ template, types: t }) { // require.resolve's paths option has been introduced in Node.js 8.9 // https://nodejs.org/api/modules.html#modules_require_resolve_request_options replacement: template({ syntacticPlaceholders: true })` - ${semverGte}(process.versions.node, "8.9") + ((a,b)=>(([,v,x,w,y]=/(\\d+)\\.(\\d+).*,(\\d+)\\.(\\d+)/.exec([a,b])),+v>+w||v==w&&+x>=+y))(process.versions.node, "8.9") ? require.resolve : (/* request */ r, { paths: [/* base */ b] }, M = require("module")) => { let /* filename */ f = M._findPath(r, M._nodeModulePaths(b).concat(b)); @@ -252,7 +252,7 @@ function pluginPolyfillsOldNode({ template, types: t }) { // fs.mkdirSync's recursive option has been introduced in Node.js 10.12 // https://nodejs.org/api/fs.html#fs_fs_mkdirsync_path_options replacement: template` - ${semverGte}(process.versions.node, "10.12") + ((a,b)=>(([,v,x,w,y]=/(\\d+)\\.(\\d+).*,(\\d+)\\.(\\d+)/.exec([a,b])),+v>+w||v==w&&+x>=+y))(process.versions.node, "10.12") ? fs.mkdirSync : require("make-dir").sync `, From 65f0fbd74e825c6c61ac94dc8abfe6ca2b7b6fb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A1ng=20J=C3=B9nli=C3=A0ng?= Date: Tue, 9 Feb 2021 11:39:25 -0500 Subject: [PATCH 3/8] don't leak variables --- babel.config.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/babel.config.js b/babel.config.js index 50de7fe39868..262551966ec0 100644 --- a/babel.config.js +++ b/babel.config.js @@ -187,14 +187,13 @@ function bool(value) { // Limitation: // - it only supports comparing major and minor version, assuming Node.js will never ship // features in patch release so we will never need to compare a version with "1.2.3" -// - it assumes `process.versions.node` never contains `,` // // @example // semverGte("8.10", "8.9") // true // semverGte("8.9", "8.9") // true // semverGte("9.0", "8.9") // true // semverGte("8.9", "8.10") // false -`((a,b)=>(([,v,x,w,y]=/(\\d+)\\.(\\d+).*,(\\d+)\\.(\\d+)/.exec([a,b])),+v>+w||v==w&&+x>=+y))`; +`((v,w)=>(v=v.split("."),w=w.split("."),+v[0]>+w[0]||v[0]==w[0]&&+v[1]>=+w[1]))`; // TODO(Babel 8) This polyfills are only needed for Node.js 6 and 8 /** @param {import("@babel/core")} api */ @@ -220,7 +219,7 @@ function pluginPolyfillsOldNode({ template, types: t }) { // require.resolve's paths option has been introduced in Node.js 8.9 // https://nodejs.org/api/modules.html#modules_require_resolve_request_options replacement: template({ syntacticPlaceholders: true })` - ((a,b)=>(([,v,x,w,y]=/(\\d+)\\.(\\d+).*,(\\d+)\\.(\\d+)/.exec([a,b])),+v>+w||v==w&&+x>=+y))(process.versions.node, "8.9") + ((v,w)=>(v=v.split("."),w=w.split("."),+v[0]>+w[0]||v[0]==w[0]&&+v[1]>=+w[1]))(process.versions.node, "8.9") ? require.resolve : (/* request */ r, { paths: [/* base */ b] }, M = require("module")) => { let /* filename */ f = M._findPath(r, M._nodeModulePaths(b).concat(b)); @@ -252,7 +251,7 @@ function pluginPolyfillsOldNode({ template, types: t }) { // fs.mkdirSync's recursive option has been introduced in Node.js 10.12 // https://nodejs.org/api/fs.html#fs_fs_mkdirsync_path_options replacement: template` - ((a,b)=>(([,v,x,w,y]=/(\\d+)\\.(\\d+).*,(\\d+)\\.(\\d+)/.exec([a,b])),+v>+w||v==w&&+x>=+y))(process.versions.node, "10.12") + ((v,w)=>(v=v.split("."),w=w.split("."),+v[0]>+w[0]||v[0]==w[0]&&+v[1]>=+w[1]))(process.versions.node, "10.12") ? fs.mkdirSync : require("make-dir").sync `, From 208c78f56360edf64fc1c8b14694e4df2903d7c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A1ng=20J=C3=B9nli=C3=A0ng?= Date: Tue, 9 Feb 2021 12:02:06 -0500 Subject: [PATCH 4/8] address reviews --- babel.config.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/babel.config.js b/babel.config.js index 262551966ec0..ff08dbe3543f 100644 --- a/babel.config.js +++ b/babel.config.js @@ -193,7 +193,9 @@ function bool(value) { // semverGte("8.9", "8.9") // true // semverGte("9.0", "8.9") // true // semverGte("8.9", "8.10") // false -`((v,w)=>(v=v.split("."),w=w.split("."),+v[0]>+w[0]||v[0]==w[0]&&+v[1]>=+w[1]))`; +// TODO: figure out how to inject it to the `@babel/template` usage so we don't need to +// copy and paste it. +// `((v,w)=>(v=v.split("."),w=w.split("."),+v[0]>+w[0]||v[0]==w[0]&&+v[1]>=+w[1]))`; // TODO(Babel 8) This polyfills are only needed for Node.js 6 and 8 /** @param {import("@babel/core")} api */ From 3741cc752c985e4a618a2b12478dd91fd1855f85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A1ng=20J=C3=B9nli=C3=A0ng?= Date: Tue, 9 Feb 2021 13:44:44 -0500 Subject: [PATCH 5/8] fix node.js 8.17 test error on Jest 24 --- packages/babel-core/test/resolution.js | 29 ++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/packages/babel-core/test/resolution.js b/packages/babel-core/test/resolution.js index aa0db96736f8..f1feddcae37a 100644 --- a/packages/babel-core/test/resolution.js +++ b/packages/babel-core/test/resolution.js @@ -334,6 +334,7 @@ describe("addon resolution", function () { }); }); + // TODO(Babel 8): remove node version check. it("should throw about module: usage for presets", function () { process.chdir("throw-module-paths"); @@ -344,7 +345,13 @@ describe("addon resolution", function () { presets: ["foo"], }); }).toThrow( - /Cannot resolve module 'babel-preset-foo'.*\n- If you want to resolve "foo", use "module:foo"/, + // Todo(Babel 8): remove node checks in this file. We cannot test the desired behaviour + // because Jest 24 has issue on setting the MODULE_NOT_FOUND error when the native + // `require.resolv` is provided. + // see https://github.com/babel/babel/pull/12439/files#r535996000 + process.versions.node.startsWith("8.") + ? /Cannot resolve module 'babel-preset-foo'/ + : /Cannot resolve module 'babel-preset-foo'.*\n- If you want to resolve "foo", use "module:foo"/, ); }); @@ -358,7 +365,9 @@ describe("addon resolution", function () { plugins: ["foo"], }); }).toThrow( - /Cannot resolve module 'babel-plugin-foo'.*\n- If you want to resolve "foo", use "module:foo"/, + process.versions.node.startsWith("8.") + ? /Cannot resolve module 'babel-plugin-foo'/ + : /Cannot resolve module 'babel-plugin-foo'.*\n- If you want to resolve "foo", use "module:foo"/, ); }); @@ -372,7 +381,9 @@ describe("addon resolution", function () { presets: ["foo"], }); }).toThrow( - /Cannot resolve module 'babel-preset-foo'.*\n- Did you mean "@babel\/foo"\?/, + process.versions.node.startsWith("8.") + ? /Cannot resolve module 'babel-preset-foo'/ + : /Cannot resolve module 'babel-preset-foo'.*\n- Did you mean "@babel\/foo"\?/, ); }); @@ -386,7 +397,9 @@ describe("addon resolution", function () { plugins: ["foo"], }); }).toThrow( - /Cannot resolve module 'babel-plugin-foo'.*\n- Did you mean "@babel\/foo"\?/, + process.versions.node.startsWith("8.") + ? /Cannot resolve module 'babel-plugin-foo'/ + : /Cannot resolve module 'babel-plugin-foo'.*\n- Did you mean "@babel\/foo"\?/, ); }); @@ -400,7 +413,9 @@ describe("addon resolution", function () { presets: ["testplugin"], }); }).toThrow( - /Cannot resolve module 'babel-preset-testplugin'.*\n- Did you accidentally pass a plugin as a preset\?/, + process.versions.node.startsWith("8.") + ? /Cannot resolve module 'babel-preset-testplugin'/ + : /Cannot resolve module 'babel-preset-testplugin'.*\n- Did you accidentally pass a plugin as a preset\?/, ); }); @@ -414,7 +429,9 @@ describe("addon resolution", function () { plugins: ["testpreset"], }); }).toThrow( - /Cannot resolve module 'babel-plugin-testpreset'.*\n- Did you accidentally pass a preset as a plugin\?/, + process.versions.node.startsWith("8.") + ? /Cannot resolve module 'babel-plugin-testpreset'/ + : /Cannot resolve module 'babel-plugin-testpreset'.*\n- Did you accidentally pass a preset as a plugin\?/, ); }); From 2db2c3accdc2e1b4991ad53de74373bfbd071466 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A1ng=20J=C3=B9nli=C3=A0ng?= Date: Tue, 9 Feb 2021 16:37:56 -0500 Subject: [PATCH 6/8] address review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Nicolò Ribaudo --- babel.config.js | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/babel.config.js b/babel.config.js index ff08dbe3543f..369799485e46 100644 --- a/babel.config.js +++ b/babel.config.js @@ -183,23 +183,21 @@ function bool(value) { return value && value !== "false" && value !== "0"; } -// A minimum semver GTE implementation -// Limitation: -// - it only supports comparing major and minor version, assuming Node.js will never ship -// features in patch release so we will never need to compare a version with "1.2.3" -// -// @example -// semverGte("8.10", "8.9") // true -// semverGte("8.9", "8.9") // true -// semverGte("9.0", "8.9") // true -// semverGte("8.9", "8.10") // false -// TODO: figure out how to inject it to the `@babel/template` usage so we don't need to -// copy and paste it. -// `((v,w)=>(v=v.split("."),w=w.split("."),+v[0]>+w[0]||v[0]==w[0]&&+v[1]>=+w[1]))`; - // TODO(Babel 8) This polyfills are only needed for Node.js 6 and 8 /** @param {import("@babel/core")} api */ function pluginPolyfillsOldNode({ template, types: t }) { + // A minimum semver GTE implementation + // Limitation: + // - it only supports comparing major and minor version, assuming Node.js will never ship + // features in patch release so we will never need to compare a version with "1.2.3" + // - the passed version must have minor version. Use "14.0" for "14" instead. + // + // @example + // semverGte("8.10", "8.9") // true + // semverGte("8.9", "8.9") // true + // semverGte("9.0", "8.9") // true + // semverGte("8.9", "8.10") // false + const semverGte = template.ast`((v,w)=>(v=v.split("."),w=w.split("."),+v[0]>+w[0]||v[0]==w[0]&&+v[1]>=+w[1]))`; const polyfills = [ { name: "require.resolve", @@ -221,7 +219,7 @@ function pluginPolyfillsOldNode({ template, types: t }) { // require.resolve's paths option has been introduced in Node.js 8.9 // https://nodejs.org/api/modules.html#modules_require_resolve_request_options replacement: template({ syntacticPlaceholders: true })` - ((v,w)=>(v=v.split("."),w=w.split("."),+v[0]>+w[0]||v[0]==w[0]&&+v[1]>=+w[1]))(process.versions.node, "8.9") + ${semverGte}(process.versions.node, "8.9") ? require.resolve : (/* request */ r, { paths: [/* base */ b] }, M = require("module")) => { let /* filename */ f = M._findPath(r, M._nodeModulePaths(b).concat(b)); @@ -253,7 +251,7 @@ function pluginPolyfillsOldNode({ template, types: t }) { // fs.mkdirSync's recursive option has been introduced in Node.js 10.12 // https://nodejs.org/api/fs.html#fs_fs_mkdirsync_path_options replacement: template` - ((v,w)=>(v=v.split("."),w=w.split("."),+v[0]>+w[0]||v[0]==w[0]&&+v[1]>=+w[1]))(process.versions.node, "10.12") + ${semverGte}(process.versions.node, "10.12") ? fs.mkdirSync : require("make-dir").sync `, From 2574c928643233b04a03fd0e22bed3ba7e315ae3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A1ng=20J=C3=B9nli=C3=A0ng?= Date: Tue, 9 Feb 2021 17:12:12 -0500 Subject: [PATCH 7/8] Revert "address review comments" This reverts commit 2db2c3accdc2e1b4991ad53de74373bfbd071466. --- babel.config.js | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/babel.config.js b/babel.config.js index 369799485e46..ff08dbe3543f 100644 --- a/babel.config.js +++ b/babel.config.js @@ -183,21 +183,23 @@ function bool(value) { return value && value !== "false" && value !== "0"; } +// A minimum semver GTE implementation +// Limitation: +// - it only supports comparing major and minor version, assuming Node.js will never ship +// features in patch release so we will never need to compare a version with "1.2.3" +// +// @example +// semverGte("8.10", "8.9") // true +// semverGte("8.9", "8.9") // true +// semverGte("9.0", "8.9") // true +// semverGte("8.9", "8.10") // false +// TODO: figure out how to inject it to the `@babel/template` usage so we don't need to +// copy and paste it. +// `((v,w)=>(v=v.split("."),w=w.split("."),+v[0]>+w[0]||v[0]==w[0]&&+v[1]>=+w[1]))`; + // TODO(Babel 8) This polyfills are only needed for Node.js 6 and 8 /** @param {import("@babel/core")} api */ function pluginPolyfillsOldNode({ template, types: t }) { - // A minimum semver GTE implementation - // Limitation: - // - it only supports comparing major and minor version, assuming Node.js will never ship - // features in patch release so we will never need to compare a version with "1.2.3" - // - the passed version must have minor version. Use "14.0" for "14" instead. - // - // @example - // semverGte("8.10", "8.9") // true - // semverGte("8.9", "8.9") // true - // semverGte("9.0", "8.9") // true - // semverGte("8.9", "8.10") // false - const semverGte = template.ast`((v,w)=>(v=v.split("."),w=w.split("."),+v[0]>+w[0]||v[0]==w[0]&&+v[1]>=+w[1]))`; const polyfills = [ { name: "require.resolve", @@ -219,7 +221,7 @@ function pluginPolyfillsOldNode({ template, types: t }) { // require.resolve's paths option has been introduced in Node.js 8.9 // https://nodejs.org/api/modules.html#modules_require_resolve_request_options replacement: template({ syntacticPlaceholders: true })` - ${semverGte}(process.versions.node, "8.9") + ((v,w)=>(v=v.split("."),w=w.split("."),+v[0]>+w[0]||v[0]==w[0]&&+v[1]>=+w[1]))(process.versions.node, "8.9") ? require.resolve : (/* request */ r, { paths: [/* base */ b] }, M = require("module")) => { let /* filename */ f = M._findPath(r, M._nodeModulePaths(b).concat(b)); @@ -251,7 +253,7 @@ function pluginPolyfillsOldNode({ template, types: t }) { // fs.mkdirSync's recursive option has been introduced in Node.js 10.12 // https://nodejs.org/api/fs.html#fs_fs_mkdirsync_path_options replacement: template` - ${semverGte}(process.versions.node, "10.12") + ((v,w)=>(v=v.split("."),w=w.split("."),+v[0]>+w[0]||v[0]==w[0]&&+v[1]>=+w[1]))(process.versions.node, "10.12") ? fs.mkdirSync : require("make-dir").sync `, From 6b8ed333a2ada9390d4403f8f996dabe88dc69d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A1ng=20J=C3=B9nli=C3=A0ng?= Date: Tue, 9 Feb 2021 17:17:34 -0500 Subject: [PATCH 8/8] typo [skip ci] --- packages/babel-core/test/resolution.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/babel-core/test/resolution.js b/packages/babel-core/test/resolution.js index f1feddcae37a..3fc2c3a2bcad 100644 --- a/packages/babel-core/test/resolution.js +++ b/packages/babel-core/test/resolution.js @@ -346,8 +346,8 @@ describe("addon resolution", function () { }); }).toThrow( // Todo(Babel 8): remove node checks in this file. We cannot test the desired behaviour - // because Jest 24 has issue on setting the MODULE_NOT_FOUND error when the native - // `require.resolv` is provided. + // because Jest 24 has an issue on setting the MODULE_NOT_FOUND error when the native + // `require.resolve` is provided. // see https://github.com/babel/babel/pull/12439/files#r535996000 process.versions.node.startsWith("8.") ? /Cannot resolve module 'babel-preset-foo'/