Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use semver gte comparison on polyfill version tester #12783

Merged
merged 8 commits into from Feb 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 16 additions & 2 deletions babel.config.js
Expand Up @@ -183,6 +183,20 @@ 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]))`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could to

const semverGte = template.ast`(v,w)=>(v=v.split("."),w=w.split("."),+v[0]>+w[0]||v[0]==w[0]&&+v[1]>=+w[1])`

And then use it with interpolations in the other templates.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok it looks like I was wrong, because we are not using template.ast here but the version that returns a factory function 😅


// TODO(Babel 8) This polyfills are only needed for Node.js 6 and 8
/** @param {import("@babel/core")} api */
function pluginPolyfillsOldNode({ template, types: t }) {
Expand All @@ -207,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 })`
parseFloat(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));
Expand Down Expand Up @@ -239,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`
parseFloat(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
`,
Expand Down
29 changes: 23 additions & 6 deletions packages/babel-core/test/resolution.js
Expand Up @@ -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");

Expand All @@ -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 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'/
: /Cannot resolve module 'babel-preset-foo'.*\n- If you want to resolve "foo", use "module:foo"/,
);
});

Expand All @@ -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"/,
);
});

Expand All @@ -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"\?/,
);
});

Expand All @@ -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"\?/,
);
});

Expand All @@ -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\?/,
);
});

Expand All @@ -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\?/,
);
});

Expand Down