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

Use require.resolve instead of the resolve package #12439

Merged
merged 3 commits into from Dec 4, 2020
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
44 changes: 44 additions & 0 deletions babel.config.js
Expand Up @@ -21,6 +21,7 @@ module.exports = function (api) {
let convertESM = true;
let ignoreLib = true;
let includeRegeneratorRuntime = false;
let polyfillRequireResolve = false;

let transformRuntimeOptions;

Expand Down Expand Up @@ -53,13 +54,15 @@ module.exports = function (api) {
"packages/babel-compat-data"
);
if (env === "rollup") envOpts.targets = { node: nodeVersion };
polyfillRequireResolve = true;
break;
case "test-legacy": // In test-legacy environment, we build babel on latest node but test on minimum supported legacy versions
case "production":
// Config during builds before publish.
envOpts.targets = {
node: nodeVersion,
};
polyfillRequireResolve = true;
break;
case "development":
envOpts.debug = true;
Expand Down Expand Up @@ -116,6 +119,7 @@ module.exports = function (api) {

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

return config;
};

// TODO(Babel 8) This polyfill is only needed for Node.js 6 and 8
function pluginPolyfillRequireResolve({ template, types: t }) {
return {
visitor: {
MemberExpression(path) {
if (!path.matchesPattern("require.resolve")) return;
if (!t.isCallExpression(path.parent, { callee: path.node })) return;

const args = path.parent.arguments;
if (args.length < 2) return;
if (
!t.isObjectExpression(args[1]) ||
args[1].properties.length !== 1 ||
!t.isIdentifier(args[1].properties[0].key, { name: "paths" }) ||
!t.isArrayExpression(args[1].properties[0].value) ||
args[1].properties[0].value.elements.length !== 1
) {
throw path.parentPath.buildCodeFrameError(
"This 'require.resolve' usage is not supported by the inline polyfill."
);
}

// require.resolve's paths option has been introduced in Node.js 8.9
// https://nodejs.org/api/modules.html#modules_require_resolve_request_options
path.replaceWith(template.ast`
parseFloat(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));
if (f) return f;
f = new Error(\`Cannot resolve module '\${r}'\`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
f = new Error(\`Cannot resolve module '\${r}'\`);
f = new Error(\`Cannot find module '\${r}'\`);

To be consistent with the native require.resolve error message.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, I put resolve here otherwise all the tests where failing. I think Jest's require.resolve uses "resolve" in its error message.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not a blocker.

@SimenB Is it possible to test MODULE_NOT_FOUND error without changing the error message? We want to make assertion about the MODULE_NOT_FOUND error but Jest seems to assume such error means the test can not be run and all tests are failing.

Copy link
Contributor

Choose a reason for hiding this comment

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

error from jest should match node, if it doesn't it's a bug

Copy link
Member Author

Choose a reason for hiding this comment

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

@JLHwung The jest version we are using on Node 6 didn't always set MODULE_NOT_FOUND anyway, so we can't test it right now.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really follow here, but if Jest 26 does something wrong with MODULE_NOT_FOUND it would be great if you opened up an issue 🙂

Copy link
Member Author

Choose a reason for hiding this comment

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

No don't worry! We are using Jest 24 on Node.js 6 and Jest 26 where it's supported. The bug was only with Jest 24, so we cannot rely on the fix but it is fixed in Jest 26!

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, good stuff 👍

f.code = "MODULE_NOT_FOUND";
throw f;
}
`);
},
},
};
}
11 changes: 5 additions & 6 deletions lib/third-party-libs.js.flow
Expand Up @@ -6,12 +6,11 @@ declare module "debug" {
declare export default (namespace: string) => (formatter: string, ...args: any[]) => void;
}

declare module "resolve" {
declare export default {
(string, {| basedir: string |}, (err: ?Error, res: string) => void): void;
sync: (string, {| basedir: string |}) => string;
};
}
declare var require: {
resolve(specifier: string, opts?: {
paths: string[]
}): string,
};

declare module "json5" {
declare export default {
Expand Down
1 change: 0 additions & 1 deletion packages/babel-core/package.json
Expand Up @@ -56,7 +56,6 @@
"gensync": "^1.0.0-beta.1",
"json5": "^2.1.2",
"lodash": "^4.17.19",
"resolve": "^1.3.2",
"semver": "^5.4.1",
"source-map": "^0.5.0"
},
Expand Down
3 changes: 1 addition & 2 deletions packages/babel-core/src/config/files/configuration.js
Expand Up @@ -17,7 +17,6 @@ import type { FilePackageData, RelativeConfig, ConfigFile } from "./types";
import type { CallerMetadata } from "../validation/options";

import * as fs from "../../gensync-utils/fs";
import resolve from "../../gensync-utils/resolve";

const debug = buildDebug("babel:config:loading:files:configuration");

Expand Down Expand Up @@ -136,7 +135,7 @@ export function* loadConfig(
envName: string,
caller: CallerMetadata | void,
): Handler<ConfigFile> {
const filepath = yield* resolve(name, { basedir: dirname });
const filepath = require.resolve(name, { paths: [dirname] });

const conf = yield* readConfig(filepath, envName, caller);
if (!conf) {
Expand Down
17 changes: 11 additions & 6 deletions packages/babel-core/src/config/files/plugins.js
Expand Up @@ -5,7 +5,6 @@
*/

import buildDebug from "debug";
import resolve from "resolve";
import path from "path";

const debug = buildDebug("babel:config:loading:files:plugins");
Expand Down Expand Up @@ -96,14 +95,18 @@ function resolveStandardizedName(
const standardizedName = standardizeName(type, name);

try {
return resolve.sync(standardizedName, { basedir: dirname });
return require.resolve(standardizedName, {
paths: [dirname],
});
} catch (e) {
if (e.code !== "MODULE_NOT_FOUND") throw e;

if (standardizedName !== name) {
let resolvedOriginal = false;
try {
resolve.sync(name, { basedir: dirname });
require.resolve(name, {
paths: [dirname],
});
resolvedOriginal = true;
} catch {}

Expand All @@ -114,8 +117,8 @@ function resolveStandardizedName(

let resolvedBabel = false;
try {
resolve.sync(standardizeName(type, "@babel/" + name), {
basedir: dirname,
require.resolve(standardizeName(type, "@babel/" + name), {
paths: [dirname],
});
resolvedBabel = true;
} catch {}
Expand All @@ -127,7 +130,9 @@ function resolveStandardizedName(
let resolvedOppositeType = false;
const oppositeType = type === "preset" ? "plugin" : "preset";
try {
resolve.sync(standardizeName(oppositeType, name), { basedir: dirname });
require.resolve(standardizeName(oppositeType, name), {
paths: [dirname],
});
resolvedOppositeType = true;
} catch {}

Expand Down
9 changes: 0 additions & 9 deletions packages/babel-core/src/gensync-utils/resolve.js

This file was deleted.

4 changes: 2 additions & 2 deletions packages/babel-core/test/api.js
Expand Up @@ -144,13 +144,13 @@ describe("parser and generator options", function () {
describe("api", function () {
it("exposes the resolvePlugin method", function () {
expect(() => babel.resolvePlugin("nonexistent-plugin")).toThrow(
/Cannot find module 'babel-plugin-nonexistent-plugin'/,
/Cannot resolve module 'babel-plugin-nonexistent-plugin'/,
);
});

it("exposes the resolvePreset method", function () {
expect(() => babel.resolvePreset("nonexistent-preset")).toThrow(
/Cannot find module 'babel-preset-nonexistent-preset'/,
/Cannot resolve module 'babel-preset-nonexistent-preset'/,
);
});

Expand Down
16 changes: 8 additions & 8 deletions packages/babel-core/test/resolution.js
Expand Up @@ -344,7 +344,7 @@ describe("addon resolution", function () {
presets: ["foo"],
});
}).toThrow(
/Cannot find module 'babel-preset-foo'.*\n- If you want to resolve "foo", use "module:foo"/,
/Cannot resolve module 'babel-preset-foo'.*\n- If you want to resolve "foo", use "module:foo"/,
);
});

Expand All @@ -358,7 +358,7 @@ describe("addon resolution", function () {
plugins: ["foo"],
});
}).toThrow(
/Cannot find module 'babel-plugin-foo'.*\n- If you want to resolve "foo", use "module:foo"/,
/Cannot resolve module 'babel-plugin-foo'.*\n- If you want to resolve "foo", use "module:foo"/,
);
});

Expand All @@ -372,7 +372,7 @@ describe("addon resolution", function () {
presets: ["foo"],
});
}).toThrow(
/Cannot find module 'babel-preset-foo'.*\n- Did you mean "@babel\/foo"\?/,
/Cannot resolve module 'babel-preset-foo'.*\n- Did you mean "@babel\/foo"\?/,
);
});

Expand All @@ -386,7 +386,7 @@ describe("addon resolution", function () {
plugins: ["foo"],
});
}).toThrow(
/Cannot find module 'babel-plugin-foo'.*\n- Did you mean "@babel\/foo"\?/,
/Cannot resolve module 'babel-plugin-foo'.*\n- Did you mean "@babel\/foo"\?/,
);
});

Expand All @@ -400,7 +400,7 @@ describe("addon resolution", function () {
presets: ["testplugin"],
});
}).toThrow(
/Cannot find module 'babel-preset-testplugin'.*\n- Did you accidentally pass a plugin as a preset\?/,
/Cannot resolve module 'babel-preset-testplugin'.*\n- Did you accidentally pass a plugin as a preset\?/,
);
});

Expand All @@ -414,7 +414,7 @@ describe("addon resolution", function () {
plugins: ["testpreset"],
});
}).toThrow(
/Cannot find module 'babel-plugin-testpreset'.*\n- Did you accidentally pass a preset as a plugin\?/,
/Cannot resolve module 'babel-plugin-testpreset'.*\n- Did you accidentally pass a preset as a plugin\?/,
);
});

Expand All @@ -427,7 +427,7 @@ describe("addon resolution", function () {
babelrc: false,
presets: ["foo"],
});
}).toThrow(/Cannot find module 'babel-preset-foo'/);
}).toThrow(/Cannot resolve module 'babel-preset-foo'/);
});

it("should throw about missing plugins", function () {
Expand All @@ -439,6 +439,6 @@ describe("addon resolution", function () {
babelrc: false,
plugins: ["foo"],
});
}).toThrow(/Cannot find module 'babel-plugin-foo'/);
}).toThrow(/Cannot resolve module 'babel-plugin-foo'/);
});
});
Expand Up @@ -23,7 +23,6 @@
"jest-diff": "^24.8.0",
"lodash": "^4.17.19",
"quick-lru": "5.1.0",
"resolve": "^1.3.2",
"source-map": "^0.5.0"
}
}
Expand Up @@ -7,7 +7,6 @@ import { codeFrameColumns } from "@babel/code-frame";
import escapeRegExp from "lodash/escapeRegExp";
import * as helpers from "./helpers";
import merge from "lodash/merge";
import resolve from "resolve";
import assert from "assert";
import fs from "fs";
import path from "path";
Expand Down Expand Up @@ -107,8 +106,8 @@ function runModuleInTestContext(
context: Context,
moduleCache: Object,
) {
const filename = resolve.sync(id, {
basedir: path.dirname(relativeFilename),
const filename = require.resolve(id, {
paths: [path.dirname(relativeFilename)],
});

// Expose Node-internal modules if the tests want them. Note, this will not execute inside
Expand Down
1 change: 0 additions & 1 deletion packages/babel-node/package.json
Expand Up @@ -29,7 +29,6 @@
"lodash": "^4.17.19",
"node-environment-flags": "^1.0.5",
"regenerator-runtime": "^0.13.4",
"resolve": "^1.13.1",
"v8flags": "^3.1.1"
},
"peerDependencies": {
Expand Down
5 changes: 2 additions & 3 deletions packages/babel-node/src/_babel-node.js
Expand Up @@ -8,7 +8,6 @@ import vm from "vm";
import "core-js/stable";
import "regenerator-runtime/runtime";
import register from "@babel/register";
import resolve from "resolve";

import pkg from "../package.json";

Expand Down Expand Up @@ -209,8 +208,8 @@ if (program.eval || program.print) {
// We have to handle require ourselves, as we want to require it in the context of babel-register
function requireArgs() {
if (program.require) {
require(resolve.sync(program.require, {
basedir: process.cwd(),
require(require.resolve(program.require, {
paths: [process.cwd()],
}));
}
}
Expand Down
Expand Up @@ -15,7 +15,7 @@ let functionPath;
transform(code, {
configFile: false,
plugins: [
"../../../../lib",
__dirname + "/../../../../lib",
{
post({ path }) {
programPath = path;
Expand Down
1 change: 0 additions & 1 deletion packages/babel-plugin-transform-runtime/package.json
Expand Up @@ -22,7 +22,6 @@
"dependencies": {
"@babel/helper-module-imports": "workspace:^7.12.1",
"@babel/helper-plugin-utils": "workspace:^7.10.4",
"resolve": "^1.8.1",
"semver": "^5.5.1"
},
"peerDependencies": {
Expand Down
@@ -1,5 +1,4 @@
import path from "path";
import resolve from "resolve";

export default function (moduleName, dirname, absoluteRuntime) {
if (absoluteRuntime === false) return moduleName;
Expand All @@ -13,7 +12,9 @@ export default function (moduleName, dirname, absoluteRuntime) {
function resolveAbsoluteRuntime(moduleName: string, dirname: string) {
try {
return path
.dirname(resolve.sync(`${moduleName}/package.json`, { basedir: dirname }))
.dirname(
require.resolve(`${moduleName}/package.json`, { paths: [dirname] }),
)
.replace(/\\/g, "/");
} catch (err) {
if (err.code !== "MODULE_NOT_FOUND") throw err;
Expand Down
@@ -1,6 +1,6 @@
var _regeneratorRuntime = require("<CWD>/node_modules/@babel/runtime-corejs3/regenerator");
var _regeneratorRuntime = require("<CWD>/packages/babel-runtime-corejs3/regenerator");

var _mapInstanceProperty = require("<CWD>/node_modules/@babel/runtime-corejs3/core-js/instance/map");
var _mapInstanceProperty = require("<CWD>/packages/babel-runtime-corejs3/core-js/instance/map");

var _marked = /*#__PURE__*/_regeneratorRuntime.mark(makeIterator);

Expand Down
@@ -1,6 +1,6 @@
var _regeneratorRuntime = require("<CWD>/node_modules/@babel/runtime-corejs3/regenerator");
var _regeneratorRuntime = require("<CWD>/packages/babel-runtime-corejs3/regenerator");

var _mapInstanceProperty = require("<CWD>/node_modules/@babel/runtime-corejs3/core-js-stable/instance/map");
var _mapInstanceProperty = require("<CWD>/packages/babel-runtime-corejs3/core-js-stable/instance/map");

var _marked = /*#__PURE__*/_regeneratorRuntime.mark(makeIterator);

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-runtime/helpers/classCallCheck");

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

function test() {
return _test.apply(this, arguments);
Expand Down
3 changes: 1 addition & 2 deletions packages/babel-plugin-transform-typeof-symbol/package.json
Expand Up @@ -26,7 +26,6 @@
"@babel/helper-plugin-test-runner": "workspace:*",
"@babel/runtime": "workspace:*",
"@babel/runtime-corejs2": "workspace:*",
"@babel/runtime-corejs3": "workspace:*",
"resolve": "^1.15.0"
"@babel/runtime-corejs3": "workspace:*"
}
}