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

Warn when the version specified in transform-runtime doesn't match #10325

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions babel.config.js
Expand Up @@ -77,6 +77,7 @@ module.exports = function(api) {
"@babel/proposal-object-rest-spread",
{ useBuiltIns: true, loose: true },
],
"@babel/proposal-optional-chaining",

// Explicitly use the lazy version of CommonJS modules.
convertESM ? ["@babel/transform-modules-commonjs", { lazy: true }] : null,
Expand Down
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -15,6 +15,7 @@
"@babel/plugin-proposal-class-properties": "^7.4.4",
"@babel/plugin-proposal-export-namespace-from": "^7.2.0",
"@babel/plugin-proposal-numeric-separator": "^7.2.0",
"@babel/plugin-proposal-optional-chaining": "^7.2.0",
"@babel/plugin-transform-modules-commonjs": "^7.4.4",
"@babel/plugin-transform-runtime": "^7.4.4",
"@babel/preset-env": "^7.4.5",
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Expand Up @@ -153,7 +153,14 @@ function buildHelper(
return babel.transformFromAst(tree, null, {
presets: [[require("@babel/preset-env"), { modules: false }]],
plugins: [
[transformRuntime, { corejs, useESModules: esm }],
[
transformRuntime,
{
corejs,
useESModules: esm,
version: require("@babel/runtime/package.json").version,
},
],
buildRuntimeRewritePlugin(
runtimeName,
path.relative(path.dirname(helperFilename), pkgDirname),
Expand Down
51 changes: 51 additions & 0 deletions packages/babel-plugin-transform-runtime/src/helpers.js
Expand Up @@ -32,6 +32,57 @@ export function hasMinVersion(minVersion, runtimeVersion) {
);
}

export function verifyRuntimeVersion(
options,
moduleName,
runtimeVersion,
actualRuntimeVersion,
) {
if (!actualRuntimeVersion) return;

const minActualVersion = semver.minVersion(actualRuntimeVersion).version;
const minVersion = semver.minVersion(runtimeVersion).version;

if (minActualVersion === minVersion) return;
Copy link
Member

Choose a reason for hiding this comment

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

Treating semver ranges as anything other than ranges is asking for trouble I think. Avoiding logic like this is why I lean toward intersects for things in

!semver.intersects(`<${minVersion}`, versionRange) &&
.

What we really want to know in this context is whether every possible version in actualRuntimeVersion is contained within runtimeVersion, I think? As in, every possible version of the runtime that could be installed by this package.json is valid given the semver range passed to the Babel plugin.

This current logic would pass if for instance we had

runtimeVersion: "^7.2.0",
actualRuntimeVersion: "~7.2.0"

since the minimum for both is 7.2.0.

I'm not sure if the semver module actually exposes and API for doing the comparison we need to do here, but we can certainly check.


const fixedOptions = JSON.stringify(
{
plugins: [
[
"@babel/plugin-transform-runtime",
{ ...options, version: minActualVersion },
],
],
},
null,
2,
).replace(/^/gm, " ".repeat(2));

if (semver.gt(minActualVersion, minVersion)) {
console.warn(
Copy link
Member

Choose a reason for hiding this comment

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

I'd be very hesitant to add new logging to a plugin in a non-major if it isn't opt-in. Some tools will treat any stderr output as an indication that something in the build has failed.

`The installed version of "${moduleName}" (${actualRuntimeVersion}) is greater ` +
`than the version specified in "@babel/transform-runtime"'s options ` +
`(${options.version}). This difference won't cause any problem in runtime ` +
`functionality, but it will make your compiled code larger since Babel ` +
`can't rely on some new or modified helpers to be present in the installed ` +
`"${moduleName}". For this reason, some helpers will be inlined in your code ` +
`as if you weren't using "@babel/plugin-transform-runtime", leadng to bigger bundles.\n` +
`To fix this problem, you can specify the correct version in ` +
`"@babel/transform-runtime"'s options:\n\n${fixedOptions}`,
);
} else {
console.warn(
`The installed version of "${moduleName}" (${actualRuntimeVersion}) is lower ` +
`than the version specified in "@babel/transform-runtime"'s options ` +
`(${options.version}). For this reason, Babel will assume that some helpers ` +
`are supported by the installed "${moduleName}" version, even if they ` +
`might not actually be present.\n` +
`To fix this problem, you must specify the correct version in ` +
`"@babel/transform-runtime"'s options:\n\n${fixedOptions}`,
);
}
}

// Note: We can't use NodePath#couldBeBaseType because it doesn't support arrays.
// Even if we added support for arrays, this package needs to be compatible with
// ^7.0.0 so we can't rely on it.
Expand Down
34 changes: 33 additions & 1 deletion packages/babel-plugin-transform-runtime/src/index.js
@@ -1,12 +1,13 @@
import path from "path";
import fs from "fs";
import resolve from "resolve";
import { declare } from "@babel/helper-plugin-utils";
import { addDefault, isModule } from "@babel/helper-module-imports";
import { types as t } from "@babel/core";

import getCoreJS2Definitions from "./runtime-corejs2-definitions";
import getCoreJS3Definitions from "./runtime-corejs3-definitions";
import { typeAnnotationToString } from "./helpers";
import { typeAnnotationToString, verifyRuntimeVersion } from "./helpers";

function resolveAbsoluteRuntime(moduleName: string, dirname: string) {
try {
Expand All @@ -27,6 +28,30 @@ function resolveAbsoluteRuntime(moduleName: string, dirname: string) {
}
}

function resolveRuntimeVersion(
moduleName: string,
modulePath: string,
dirname: string,
) {
try {
const runtimeDir = path.dirname(
resolve.sync(`${modulePath}/package.json`, {
basedir: dirname,
}),
);
const pkgFilename = path.resolve(runtimeDir, "../../../package.json");
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I'm not sure this is what I'd recommend for verifying this. Could we consider looking for the package.json in a parent directory starting from dirname? It seems like treating the root as relative to the plugin usage location might be safer than using the location of the plugin, since that could be in an unknown place.

const pkg = JSON.parse(fs.readFileSync(pkgFilename, "utf8"));

return (
pkg.dependencies?.[moduleName] ||
Copy link
Member Author

Choose a reason for hiding this comment

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

It is stage 3 now 🙏

pkg.devDependencies?.[moduleName] ||
pkg.peerDependencies?.[moduleName]
);
} catch {}

return null;
}

function supportsStaticESM(caller) {
return !!(caller && caller.supportsStaticESM);
}
Expand Down Expand Up @@ -189,6 +214,13 @@ export default declare((api, options, dirname) => {
);
}

verifyRuntimeVersion(
options,
moduleName,
runtimeVersion,
resolveRuntimeVersion(moduleName, modulePath, dirname),
);

return {
name: "transform-runtime",

Expand Down
@@ -1,4 +1,4 @@
var _classCallCheck = require("<CWD>/packages/babel-plugin-transform-runtime/node_modules/@babel/runtime/helpers/classCallCheck");
var _classCallCheck = require("<CWD>/packages/babel-plugin-transform-runtime/test/node_modules/@babel/runtime/helpers/classCallCheck");
JLHwung marked this conversation as resolved.
Show resolved Hide resolved

let Foo = function Foo() {
"use strict";
Expand Down
@@ -1,4 +1,4 @@
var _classCallCheck = require("<CWD>/packages/babel-plugin-transform-runtime/node_modules/@babel/runtime/helpers/classCallCheck");
var _classCallCheck = require("<CWD>/packages/babel-plugin-transform-runtime/test/node_modules/@babel/runtime/helpers/classCallCheck");

let Foo = function Foo() {
"use strict";
Expand Down
@@ -0,0 +1 @@
;

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

@@ -0,0 +1,4 @@
{
"validateLogs": true,
"plugins": [["transform-runtime", { "version": "7.2.0" }]]
}
@@ -0,0 +1 @@
;
@@ -0,0 +1,5 @@
{
"dependencies": {
"@babel/runtime": "^7.3.0"
}
}
@@ -0,0 +1,13 @@
The installed version of "@babel/runtime" (^7.3.0) is greater than the version specified in "@babel/transform-runtime"'s options (7.2.0). This difference won't cause any problem in runtime functionality, but it will make your compiled code larger since Babel can't rely on some new or modified helpers to be present in the installed "@babel/runtime". For this reason, some helpers will be inlined in your code as if you weren't using "@babel/plugin-transform-runtime", leadng to bigger bundles.
To fix this problem, you can specify the correct version in "@babel/transform-runtime"'s options:

{
"plugins": [
[
"@babel/plugin-transform-runtime",
{
"version": "7.3.0"
}
]
]
}
@@ -0,0 +1 @@
;

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

@@ -0,0 +1,4 @@
{
"validateLogs": true,
"plugins": [["@babel/plugin-transform-runtime", { "version": "7.5.0" }]]
}
@@ -0,0 +1 @@
;
@@ -0,0 +1,5 @@
{
"dependencies": {
"@babel/runtime": "^7.3.0"
}
}
@@ -0,0 +1,13 @@
The installed version of "@babel/runtime" (^7.3.0) is lower than the version specified in "@babel/transform-runtime"'s options (7.5.0). For this reason, Babel will assume that some helpers are supported by the installed "@babel/runtime" version, even if they might not actually be present.
To fix this problem, you must specify the correct version in "@babel/transform-runtime"'s options:

{
"plugins": [
[
"@babel/plugin-transform-runtime",
{
"version": "7.3.0"
}
]
]
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 15 additions & 0 deletions yarn.lock
Expand Up @@ -317,6 +317,14 @@
"@babel/helper-plugin-utils" "^7.0.0"
"@babel/plugin-syntax-optional-catch-binding" "^7.2.0"

"@babel/plugin-proposal-optional-chaining@^7.2.0":
version "7.2.0"
resolved "https://registry.yarnpkg.com/@babel/plugin-proposal-optional-chaining/-/plugin-proposal-optional-chaining-7.2.0.tgz#ae454f4c21c6c2ce8cb2397dc332ae8b420c5441"
integrity sha512-ea3Q6edZC/55wEBVZAEz42v528VulyO0eir+7uky/sT4XRcdkWJcFi1aPtitTlwUzGnECWJNExWww1SStt+yWw==
dependencies:
"@babel/helper-plugin-utils" "^7.0.0"
"@babel/plugin-syntax-optional-chaining" "^7.2.0"

"@babel/plugin-proposal-unicode-property-regex@^7.4.4":
version "7.4.4"
resolved "https://registry.yarnpkg.com/@babel/plugin-proposal-unicode-property-regex/-/plugin-proposal-unicode-property-regex-7.4.4.tgz#501ffd9826c0b91da22690720722ac7cb1ca9c78"
Expand Down Expand Up @@ -382,6 +390,13 @@
dependencies:
"@babel/helper-plugin-utils" "^7.0.0"

"@babel/plugin-syntax-optional-chaining@^7.2.0":
version "7.2.0"
resolved "https://registry.yarnpkg.com/@babel/plugin-syntax-optional-chaining/-/plugin-syntax-optional-chaining-7.2.0.tgz#a59d6ae8c167e7608eaa443fda9fa8fa6bf21dff"
integrity sha512-HtGCtvp5Uq/jH/WNUPkK6b7rufnCPLLlDAFN7cmACoIjaOOiXxUt3SswU5loHqrhtqTsa/WoLQ1OQ1AGuZqaWA==
dependencies:
"@babel/helper-plugin-utils" "^7.0.0"

"@babel/plugin-transform-arrow-functions@^7.2.0":
version "7.2.0"
resolved "https://registry.yarnpkg.com/@babel/plugin-transform-arrow-functions/-/plugin-transform-arrow-functions-7.2.0.tgz#9aeafbe4d6ffc6563bf8f8372091628f00779550"
Expand Down