Skip to content

Commit

Permalink
improve resolving of build dependencies when exports field is used
Browse files Browse the repository at this point in the history
  • Loading branch information
sokra committed Apr 6, 2021
1 parent cffa1ad commit e2fb89e
Show file tree
Hide file tree
Showing 11 changed files with 74 additions and 10 deletions.
62 changes: 52 additions & 10 deletions lib/FileSystemInfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,12 @@ const RBDT_RESOLVE_CJS = 0;
const RBDT_RESOLVE_ESM = 1;
const RBDT_RESOLVE_DIRECTORY = 2;
const RBDT_RESOLVE_CJS_FILE = 3;
const RBDT_RESOLVE_ESM_FILE = 4;
const RBDT_DIRECTORY = 5;
const RBDT_FILE = 6;
const RBDT_DIRECTORY_DEPENDENCIES = 7;
const RBDT_FILE_DEPENDENCIES = 8;
const RBDT_RESOLVE_CJS_FILE_AS_CHILD = 4;
const RBDT_RESOLVE_ESM_FILE = 5;
const RBDT_DIRECTORY = 6;
const RBDT_FILE = 7;
const RBDT_DIRECTORY_DEPENDENCIES = 8;
const RBDT_FILE_DEPENDENCIES = 9;

const INVALID = Symbol("invalid");

Expand Down Expand Up @@ -1103,15 +1104,23 @@ class FileSystemInfo {
const resolveCjs = createResolver({
extensions: [".js", ".json", ".node"],
conditionNames: ["require", "node"],
exportsFields: ["exports"],
fileSystem: this.fs
});
const resolveCjsAsChild = createResolver({
extensions: [".js", ".json", ".node"],
conditionNames: ["require", "node"],
exportsFields: [],
fileSystem: this.fs
});
const resolveEsm = createResolver({
extensions: [".js", ".json", ".node"],
fullySpecified: true,
conditionNames: ["import", "node"],
exportsFields: ["exports"],
fileSystem: this.fs
});
return { resolveContext, resolveEsm, resolveCjs };
return { resolveContext, resolveEsm, resolveCjs, resolveCjsAsChild };
}

/**
Expand All @@ -1124,7 +1133,8 @@ class FileSystemInfo {
const {
resolveContext,
resolveEsm,
resolveCjs
resolveCjs,
resolveCjsAsChild
} = this._createBuildDependenciesResolvers();

/** @type {Set<string>} */
Expand Down Expand Up @@ -1242,7 +1252,7 @@ class FileSystemInfo {
resolveResults.set(key, result);
} else {
invalidResolveResults.add(key);
this.logger.debug(
this.logger.warn(
`Resolving '${path}' in ${context} for build dependencies doesn't lead to expected result '${expected}', but to '${result}' instead. Resolving dependencies are ignored for this path.\n${pathToString(
job
)}`
Expand Down Expand Up @@ -1299,6 +1309,10 @@ class FileSystemInfo {
resolveFile(path, "f", resolveCjs);
break;
}
case RBDT_RESOLVE_CJS_FILE_AS_CHILD: {
resolveFile(path, "c", resolveCjsAsChild);
break;
}
case RBDT_RESOLVE_ESM_FILE: {
resolveFile(path, "e", resolveEsm);
break;
Expand Down Expand Up @@ -1378,11 +1392,29 @@ class FileSystemInfo {
const context = dirname(this.fs, path);
for (const modulePath of module.paths) {
if (childPath.startsWith(modulePath)) {
let request = childPath.slice(modulePath.length + 1);
let subPath = childPath.slice(modulePath.length + 1);
const packageMatch = /^(@[^\\/]+[\\/])[^\\/]+/.exec(
subPath
);
if (packageMatch) {
push({
type: RBDT_FILE,
context: undefined,
path:
modulePath +
childPath[modulePath.length] +
packageMatch[0] +
childPath[modulePath.length] +
"package.json",
expected: false,
issuer: job
});
}
let request = subPath.replace(/\\/g, "/");
if (request.endsWith(".js"))
request = request.slice(0, -3);
push({
type: RBDT_RESOLVE_CJS_FILE,
type: RBDT_RESOLVE_CJS_FILE_AS_CHILD,
context,
path: request,
expected: child.filename,
Expand Down Expand Up @@ -1573,6 +1605,7 @@ class FileSystemInfo {
checkResolveResultsValid(resolveResults, callback) {
const {
resolveCjs,
resolveCjsAsChild,
resolveEsm,
resolveContext
} = this._createBuildDependenciesResolvers();
Expand Down Expand Up @@ -1600,6 +1633,15 @@ class FileSystemInfo {
callback();
});
break;
case "c":
resolveCjsAsChild(context, path, {}, (err, result) => {
if (expectedResult === false)
return callback(err ? undefined : INVALID);
if (err) return callback(err);
if (result !== expectedResult) return callback(INVALID);
callback();
});
break;
case "e":
resolveEsm(context, path, {}, (err, result) => {
if (expectedResult === false)
Expand Down
4 changes: 4 additions & 0 deletions test/BuildDependencies.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,14 @@ describe("BuildDependencies", () => {
const output2 = await exec("2");
expect(output2).toMatch(/but build dependencies have changed/);
expect(output2).toMatch(/Captured build dependencies/);
expect(output2).not.toMatch(/Assuming/);
expect(output2).not.toMatch(/<w>/);
const output3 = await exec("3");
expect(output3).not.toMatch(/resolving of build dependencies is invalid/);
expect(output3).not.toMatch(/but build dependencies have changed/);
expect(output3).not.toMatch(/Captured build dependencies/);
expect(output3).not.toMatch(/Assuming/);
expect(output3).not.toMatch(/<w>/);
fs.writeFileSync(
path.resolve(inputDirectory, "package.json"),
JSON.stringify({
Expand Down
Empty file.

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

Empty file.
Empty file.
Empty file.

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.

2 changes: 2 additions & 0 deletions test/fixtures/buildDependencies/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const options = JSON.parse(process.argv[3]);
const esm = +process.versions.modules >= 83;

if (esm) {
require("require-dependency-with-exports");
import("./esm.mjs").then(module => {
run(module);
});
Expand Down Expand Up @@ -56,6 +57,7 @@ function run({ default: value2, asyncDep: value3 }) {
type: "filesystem",
cacheDirectory: path.resolve(__dirname, "../../js/buildDepsCache"),
buildDependencies: {
defaultWebpack: [],
config: [
__filename,
path.resolve(__dirname, "../../../node_modules/.yarn-integrity")
Expand Down

0 comments on commit e2fb89e

Please sign in to comment.