From e2fb89eed19264e4f45c144bd483f2e03eef5c4f Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Tue, 6 Apr 2021 13:58:36 +0200 Subject: [PATCH] improve resolving of build dependencies when `exports` field is used --- lib/FileSystemInfo.js | 62 ++++++++++++++++--- test/BuildDependencies.test.js | 4 ++ .../dep-with-exports/main-entry.js | 0 .../dep-with-exports/package.json | 8 +++ .../dep-with-exports/sub-entry.js | 0 .../dep-without-package.json/main-entry.js | 0 .../dep-without-package.json/sub-entry.js | 0 .../dependency-with-exports/main.js | 4 ++ .../dependency-with-exports/package.json | 3 + .../require-dependency-with-exports/index.js | 1 + test/fixtures/buildDependencies/run.js | 2 + 11 files changed, 74 insertions(+), 10 deletions(-) create mode 100644 test/fixtures/buildDependencies/node_modules/dep-with-exports/main-entry.js create mode 100644 test/fixtures/buildDependencies/node_modules/dep-with-exports/package.json create mode 100644 test/fixtures/buildDependencies/node_modules/dep-with-exports/sub-entry.js create mode 100644 test/fixtures/buildDependencies/node_modules/dep-without-package.json/main-entry.js create mode 100644 test/fixtures/buildDependencies/node_modules/dep-without-package.json/sub-entry.js create mode 100644 test/fixtures/buildDependencies/node_modules/dependency-with-exports/main.js create mode 100644 test/fixtures/buildDependencies/node_modules/dependency-with-exports/package.json create mode 100644 test/fixtures/buildDependencies/node_modules/require-dependency-with-exports/index.js diff --git a/lib/FileSystemInfo.js b/lib/FileSystemInfo.js index 70fcb0226..1712f1fb1 100644 --- a/lib/FileSystemInfo.js +++ b/lib/FileSystemInfo.js @@ -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"); @@ -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 }; } /** @@ -1124,7 +1133,8 @@ class FileSystemInfo { const { resolveContext, resolveEsm, - resolveCjs + resolveCjs, + resolveCjsAsChild } = this._createBuildDependenciesResolvers(); /** @type {Set} */ @@ -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 )}` @@ -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; @@ -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, @@ -1573,6 +1605,7 @@ class FileSystemInfo { checkResolveResultsValid(resolveResults, callback) { const { resolveCjs, + resolveCjsAsChild, resolveEsm, resolveContext } = this._createBuildDependenciesResolvers(); @@ -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) diff --git a/test/BuildDependencies.test.js b/test/BuildDependencies.test.js index c64016c45..e65b2d1db 100644 --- a/test/BuildDependencies.test.js +++ b/test/BuildDependencies.test.js @@ -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(//); 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(//); fs.writeFileSync( path.resolve(inputDirectory, "package.json"), JSON.stringify({ diff --git a/test/fixtures/buildDependencies/node_modules/dep-with-exports/main-entry.js b/test/fixtures/buildDependencies/node_modules/dep-with-exports/main-entry.js new file mode 100644 index 000000000..e69de29bb diff --git a/test/fixtures/buildDependencies/node_modules/dep-with-exports/package.json b/test/fixtures/buildDependencies/node_modules/dep-with-exports/package.json new file mode 100644 index 000000000..021879e7c --- /dev/null +++ b/test/fixtures/buildDependencies/node_modules/dep-with-exports/package.json @@ -0,0 +1,8 @@ +{ + "exports": { + ".": "./main-entry.js", + "./sub": { + "require": "./sub-entry.js" + } + } +} diff --git a/test/fixtures/buildDependencies/node_modules/dep-with-exports/sub-entry.js b/test/fixtures/buildDependencies/node_modules/dep-with-exports/sub-entry.js new file mode 100644 index 000000000..e69de29bb diff --git a/test/fixtures/buildDependencies/node_modules/dep-without-package.json/main-entry.js b/test/fixtures/buildDependencies/node_modules/dep-without-package.json/main-entry.js new file mode 100644 index 000000000..e69de29bb diff --git a/test/fixtures/buildDependencies/node_modules/dep-without-package.json/sub-entry.js b/test/fixtures/buildDependencies/node_modules/dep-without-package.json/sub-entry.js new file mode 100644 index 000000000..e69de29bb diff --git a/test/fixtures/buildDependencies/node_modules/dependency-with-exports/main.js b/test/fixtures/buildDependencies/node_modules/dependency-with-exports/main.js new file mode 100644 index 000000000..af238348f --- /dev/null +++ b/test/fixtures/buildDependencies/node_modules/dependency-with-exports/main.js @@ -0,0 +1,4 @@ +require("dep-with-exports"); +require("dep-with-exports/sub"); +require("dep-without-package.json/main-entry"); +require("dep-without-package.json/sub-entry"); diff --git a/test/fixtures/buildDependencies/node_modules/dependency-with-exports/package.json b/test/fixtures/buildDependencies/node_modules/dependency-with-exports/package.json new file mode 100644 index 000000000..5ebad0b4b --- /dev/null +++ b/test/fixtures/buildDependencies/node_modules/dependency-with-exports/package.json @@ -0,0 +1,3 @@ +{ + "exports": "./main.js" +} diff --git a/test/fixtures/buildDependencies/node_modules/require-dependency-with-exports/index.js b/test/fixtures/buildDependencies/node_modules/require-dependency-with-exports/index.js new file mode 100644 index 000000000..055d4f99e --- /dev/null +++ b/test/fixtures/buildDependencies/node_modules/require-dependency-with-exports/index.js @@ -0,0 +1 @@ +require("dependency-with-exports"); diff --git a/test/fixtures/buildDependencies/run.js b/test/fixtures/buildDependencies/run.js index 2c137b844..e6aeb0a13 100644 --- a/test/fixtures/buildDependencies/run.js +++ b/test/fixtures/buildDependencies/run.js @@ -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); }); @@ -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")