From fd86859fa0d71c6e614eb0df58749f939bde3df6 Mon Sep 17 00:00:00 2001 From: "alexander.akait" Date: Fri, 19 Jul 2024 01:45:09 +0300 Subject: [PATCH 1/2] fix: `exports` and `imports` array target resolving --- lib/ExportsFieldPlugin.js | 16 +- lib/ImportsFieldPlugin.js | 28 ++- lib/forEachBail.js | 4 +- test/__snapshots__/exportsField.test.js.snap | 3 - test/__snapshots__/importsField.test.js.snap | 8 - test/exportsField.test.js | 171 +++++++++++++++++- .../package.json | 26 ++- .../node_modules/exports-field/package.json | 3 +- .../node_modules/exports-field/package.json | 2 +- .../imports-field-different/package.json | 7 +- .../imports-field/node_modules/a/package.json | 2 +- test/importsField.test.js | 71 +++++++- 12 files changed, 294 insertions(+), 47 deletions(-) diff --git a/lib/ExportsFieldPlugin.js b/lib/ExportsFieldPlugin.js index 1c45b888..671c1fee 100644 --- a/lib/ExportsFieldPlugin.js +++ b/lib/ExportsFieldPlugin.js @@ -125,9 +125,10 @@ module.exports = class ExportsFieldPlugin { /** * @param {string} p path * @param {(err?: null|Error, result?: null|ResolveRequest) => void} callback callback + * @param {number} i index * @returns {void} */ - (p, callback) => { + (p, callback, i) => { const parsedIdentifier = parseIdentifier(p); if (!parsedIdentifier) return callback(); @@ -135,7 +136,7 @@ module.exports = class ExportsFieldPlugin { const [relativePath, query, fragment] = parsedIdentifier; if (relativePath.length === 0 || !relativePath.startsWith("./")) { - if (paths.length === 1) { + if (paths.length === i) { return callback( new Error( `Invalid "exports" target "${p}" defined for "${usedField}" in the package config ${request.descriptionFilePath}, targets must start with "./"` @@ -150,10 +151,10 @@ module.exports = class ExportsFieldPlugin { invalidSegmentRegEx.exec(relativePath.slice(2)) !== null && deprecatedInvalidSegmentRegEx.test(relativePath.slice(2)) !== null ) { - if (paths.length === 1) { + if (paths.length === i) { return callback( new Error( - `Trying to access out of package scope. Requesting ${relativePath}` + `Invalid "exports" target "${p}" defined for "${usedField}" in the package config ${request.descriptionFilePath}, targets must start with "./"` ) ); } @@ -179,7 +180,12 @@ module.exports = class ExportsFieldPlugin { obj, "using exports field: " + p, resolveContext, - callback + (err, result) => { + if (err) return callback(err); + // Don't allow to continue - https://github.com/webpack/enhanced-resolve/issues/400 + if (result === undefined) return callback(null, null); + callback(null, result); + } ); }, /** diff --git a/lib/ImportsFieldPlugin.js b/lib/ImportsFieldPlugin.js index 56462b31..15cad76f 100644 --- a/lib/ImportsFieldPlugin.js +++ b/lib/ImportsFieldPlugin.js @@ -85,6 +85,8 @@ module.exports = class ImportsFieldPlugin { /** @type {string[]} */ let paths; + /** @type {string | null} */ + let usedField; try { // We attach the cache to the description file instead of the importsField value @@ -100,7 +102,10 @@ module.exports = class ImportsFieldPlugin { fieldProcessor ); } - [paths] = fieldProcessor(remainingRequest, this.conditionNames); + [paths, usedField] = fieldProcessor( + remainingRequest, + this.conditionNames + ); } catch (/** @type {unknown} */ err) { if (resolveContext.log) { resolveContext.log( @@ -123,9 +128,10 @@ module.exports = class ImportsFieldPlugin { /** * @param {string} p path * @param {(err?: null|Error, result?: null|ResolveRequest) => void} callback callback + * @param {number} i index * @returns {void} */ - (p, callback) => { + (p, callback, i) => { const parsedIdentifier = parseIdentifier(p); if (!parsedIdentifier) return callback(); @@ -139,10 +145,10 @@ module.exports = class ImportsFieldPlugin { invalidSegmentRegEx.exec(path_.slice(2)) !== null && deprecatedInvalidSegmentRegEx.test(path_.slice(2)) !== null ) { - if (paths.length === 1) { + if (paths.length === i) { return callback( new Error( - `Trying to access out of package scope. Requesting ${path_}` + `Invalid "imports" target "${p}" defined for "${usedField}" in the package config ${request.descriptionFilePath}, targets must start with "./"` ) ); } @@ -168,7 +174,12 @@ module.exports = class ImportsFieldPlugin { obj, "using imports field: " + p, resolveContext, - callback + (err, result) => { + if (err) return callback(err); + // Don't allow to continue - https://github.com/webpack/enhanced-resolve/issues/400 + if (result === undefined) return callback(null, null); + callback(null, result); + } ); break; } @@ -190,7 +201,12 @@ module.exports = class ImportsFieldPlugin { obj, "using imports field: " + p, resolveContext, - callback + (err, result) => { + if (err) return callback(err); + // Don't allow to continue - https://github.com/webpack/enhanced-resolve/issues/400 + if (result === undefined) return callback(null, null); + callback(null, result); + } ); } } diff --git a/lib/forEachBail.js b/lib/forEachBail.js index 273afff8..32a7250a 100644 --- a/lib/forEachBail.js +++ b/lib/forEachBail.js @@ -22,7 +22,7 @@ * @template Z * @param {T[]} array array * @param {Iterator} iterator iterator - * @param {(err?: null|Error, result?: null|Z) => void} callback callback after all items are iterated + * @param {(err?: null|Error, result?: null|Z, i?: number) => void} callback callback after all items are iterated * @returns {void} */ module.exports = function forEachBail(array, iterator, callback) { @@ -36,7 +36,7 @@ module.exports = function forEachBail(array, iterator, callback) { array[i++], (err, result) => { if (err || result !== undefined || i >= array.length) { - return callback(err, result); + return callback(err, result, i); } if (loop === false) while (next()); loop = true; diff --git a/test/__snapshots__/exportsField.test.js.snap b/test/__snapshots__/exportsField.test.js.snap index fa62e152..eae41b10 100644 --- a/test/__snapshots__/exportsField.test.js.snap +++ b/test/__snapshots__/exportsField.test.js.snap @@ -9,9 +9,6 @@ Array [ " looking for modules in .../node_modules", " existing directory .../node_modules/exports-field", " using description file: .../node_modules/exports-field/package.json (relative path: .)", - " using exports field: ./lib/lib2/browser.js", - " using description file: .../node_modules/exports-field/package.json (relative path: ./lib/lib2/browser.js)", - " .../node_modules/exports-field/lib/lib2/browser.js doesn't exist", " using exports field: ./lib/browser.js", " using description file: .../node_modules/exports-field/package.json (relative path: ./lib/browser.js)", " existing file: .../node_modules/exports-field/lib/browser.js", diff --git a/test/__snapshots__/importsField.test.js.snap b/test/__snapshots__/importsField.test.js.snap index 19b4c67c..aff48610 100644 --- a/test/__snapshots__/importsField.test.js.snap +++ b/test/__snapshots__/importsField.test.js.snap @@ -12,14 +12,6 @@ Array [ " looking for modules in .../node_modules", " existing directory .../node_modules/a", " using description file: .../node_modules/a/package.json (relative path: .)", - " using exports field: ./lib/lib2/index.js", - " using description file: .../node_modules/a/package.json (relative path: ./lib/lib2/index.js)", - " no extension", - " .../node_modules/a/lib/lib2/index.js doesn't exist", - " .js", - " .../node_modules/a/lib/lib2/index.js.js doesn't exist", - " as directory", - " .../node_modules/a/lib/lib2/index.js doesn't exist", " using exports field: ./lib/index.js", " using description file: .../node_modules/a/package.json (relative path: ./lib/index.js)", " no extension", diff --git a/test/exportsField.test.js b/test/exportsField.test.js index 016dc2ac..5ec5d4e9 100644 --- a/test/exportsField.test.js +++ b/test/exportsField.test.js @@ -2181,7 +2181,7 @@ describe("ExportsFieldPlugin", () => { it("resolve using exports field, not a browser field #1", done => { const resolver = ResolverFactory.createResolver({ aliasFields: ["browser"], - conditionNames: ["webpack"], + conditionNames: ["custom"], extensions: [".js"], fileSystem: nodeFileSystem }); @@ -2205,9 +2205,9 @@ describe("ExportsFieldPlugin", () => { it("resolve using exports field and a browser alias field #2", done => { const resolver = ResolverFactory.createResolver({ aliasFields: ["browser"], + conditionNames: ["node"], extensions: [".js"], - fileSystem: nodeFileSystem, - conditionNames: ["node"] + fileSystem: nodeFileSystem }); resolver.resolve( @@ -2266,7 +2266,7 @@ describe("ExportsFieldPlugin", () => { if (err) return done(err); if (!result) return done(new Error("No result")); expect(result).toEqual( - path.resolve(fixture2, "node_modules/exports-field/lib/lib2/main.js") + path.resolve(fixture2, "node_modules/exports-field/lib/main.js") ); done(); } @@ -2283,7 +2283,7 @@ describe("ExportsFieldPlugin", () => { if (err) return done(err); if (!result) return done(new Error("No result")); expect(result).toEqual( - path.resolve(fixture, "node_modules/exports-field/lib/lib2/main.js") + path.resolve(fixture, "node_modules/exports-field/lib/main.js") ); done(); } @@ -2406,7 +2406,9 @@ describe("ExportsFieldPlugin", () => { (err, result) => { if (!err) return done(new Error(`expect error, got ${result}`)); expect(err).toBeInstanceOf(Error); - expect(err.message).toMatch(/Can't resolve/); + expect(err.message).toMatch( + /Invalid "exports" target "\.\/lib\/lib2\/\.\.\/\.\.\/\.\.\/a\.js" defined for "\.\/dist\/"/ + ); done(); } ); @@ -2421,7 +2423,9 @@ describe("ExportsFieldPlugin", () => { (err, result) => { if (!err) return done(new Error(`expect error, got ${result}`)); expect(err).toBeInstanceOf(Error); - expect(err.message).toMatch(/out of package scope/); + expect(err.message).toMatch( + /Invalid "exports" target "\.\/\.\.\/\.\.\/a\.js" defined for "\.\/dist\/a\.js"/ + ); done(); } ); @@ -2572,7 +2576,9 @@ describe("ExportsFieldPlugin", () => { resolver.resolve({}, fixture4, "exports-field", {}, (err, result) => { if (!err) return done(new Error(`expect error, got ${result}`)); expect(err).toBeInstanceOf(Error); - expect(err.message).toMatch(/Trying to access out of package scope/); + expect(err.message).toMatch( + /Invalid "exports" target "\.\/a\/\.\.\/b\/\.\.\/\.\.\/pack1\/index\.js" defined for "\."/ + ); done(); }); }); @@ -3000,7 +3006,9 @@ describe("ExportsFieldPlugin", () => { (err, result) => { if (!err) return done(new Error(`expect error, got ${result}`)); expect(err).toBeInstanceOf(Error); - expect(err.message).toMatch(/Can't resolve/); + expect(err.message).toMatch( + /Invalid "exports" target "foo" defined for "\.\/baz-multi"/ + ); done(); } ); @@ -3169,4 +3177,149 @@ describe("ExportsFieldPlugin", () => { } ); }); + + it("invalid package target #15", done => { + resolver.resolve( + {}, + fixture5, + "@exports-field/bad-specifier/non-existent.js", + {}, + (err, result) => { + if (!err) return done(new Error(`expect error, got ${result}`)); + expect(err).toBeInstanceOf(Error); + expect(err.message).toMatch( + /Can't resolve '@exports-field\/bad-specifier\/non-existent\.js'/ + ); + done(); + } + ); + }); + + it("invalid package target #16", done => { + resolver.resolve( + {}, + fixture5, + "@exports-field/bad-specifier/dep/multi1", + {}, + (err, result) => { + if (!err) return done(new Error(`expect error, got ${result}`)); + expect(err).toBeInstanceOf(Error); + expect(err.message).toMatch( + /Invalid "exports" target "\.\.\/\.\.\/test\/foo" defined for "\.\/dep\/multi1"/ + ); + done(); + } + ); + }); + + it("invalid package target #17", done => { + resolver.resolve( + {}, + fixture5, + "@exports-field/bad-specifier/dep/multi2", + {}, + (err, result) => { + if (!err) return done(new Error(`expect error, got ${result}`)); + expect(err).toBeInstanceOf(Error); + expect(err.message).toMatch( + /Invalid "exports" target "\.\.\/\.\.\/test" defined for "\.\/dep\/multi2"/ + ); + done(); + } + ); + }); + + it("invalid package target #18", done => { + resolver.resolve( + {}, + fixture5, + "@exports-field/bad-specifier/dep/multi4", + {}, + (err, result) => { + if (!err) return done(new Error(`expect error, got ${result}`)); + expect(err).toBeInstanceOf(Error); + expect(err.message).toMatch( + /Invalid "exports" target "\.\/c\/\.\.\/b\/\.\.\/\.\.\/pack1\/index\.js" defined for "\.\/dep\/multi4"/ + ); + done(); + } + ); + }); + + it("invalid package target #19", done => { + resolver.resolve( + {}, + fixture5, + "@exports-field/bad-specifier/dep/multi5", + {}, + (err, result) => { + if (!err) return done(new Error(`expect error, got ${result}`)); + expect(err).toBeInstanceOf(Error); + expect(err.message).toMatch( + /Invalid "exports" target "\.\/a\/\.\.\/b\/\.\.\/\.\.\/pack1\/index\.js" defined for "\.\/dep\/multi5"/ + ); + done(); + } + ); + }); + + it("should resolve the valid thing in array of export #1", done => { + resolver.resolve( + {}, + fixture5, + "@exports-field/bad-specifier/bad-specifier.js", + {}, + (err, result) => { + if (err) return done(err); + if (!result) return done(new Error("No result")); + expect(result).toEqual(path.resolve(fixture5, "./a.js")); + done(); + } + ); + }); + + it("should resolve the valid thing in array of export #2", done => { + resolver.resolve( + {}, + fixture5, + "@exports-field/bad-specifier/bad-specifier1.js", + {}, + (err, result) => { + if (err) return done(err); + if (!result) return done(new Error("No result")); + expect(result).toEqual(path.resolve(fixture5, "./a.js")); + done(); + } + ); + }); + + it("should resolve the valid thing in array of export #3", done => { + resolver.resolve( + {}, + fixture5, + "@exports-field/bad-specifier/dep/multi", + {}, + (err, result) => { + if (err) return done(err); + if (!result) return done(new Error("No result")); + expect(result).toEqual(path.resolve(fixture5, "./a.js")); + done(); + } + ); + }); + + it("should resolve the valid thing in array of export #4", done => { + resolver.resolve( + {}, + fixture5, + "@exports-field/bad-specifier/dep/multi3", + {}, + (err, result) => { + if (err) return done(err); + if (!result) return done(new Error("No result")); + expect(result).toEqual(path.resolve(fixture5, "./a.js")); + done(); + } + ); + }); }); diff --git a/test/fixtures/exports-field-invalid-package-target/package.json b/test/fixtures/exports-field-invalid-package-target/package.json index d0aa09e5..248677c7 100644 --- a/test/fixtures/exports-field-invalid-package-target/package.json +++ b/test/fixtures/exports-field-invalid-package-target/package.json @@ -39,6 +39,30 @@ }, "./utils4/*": "../src/*", "./utils5/": "../src/", - "./*": "." + "./*": ".", + "./valid/*.js": { + "default": ["-bad-specifier-", "./*.js"] + }, + "./non-existent.js": [ + "-bad-specifier-", + "./non-existent.js", + "./a.js" + ], + "./bad-specifier.js": [ + "-bad-specifier-", + "../../a.js", + "./a.js" + ], + "./bad-specifier1.js": [ + "-bad-specifier-", + "foo", + "./a.js" + ], + "./dep/multi": ["../../test", "./a.js"], + "./dep/multi1": ["../../test", "../../test/foo"], + "./dep/multi2": ["../../test"], + "./dep/multi3": ["./a/../b/../../pack1/index.js", "./a.js"], + "./dep/multi4": ["./a/../b/../../pack1/index.js", "./c/../b/../../pack1/index.js"], + "./dep/multi5": ["./a/../b/../../pack1/index.js"] } } diff --git a/test/fixtures/exports-field/node_modules/exports-field/package.json b/test/fixtures/exports-field/node_modules/exports-field/package.json index 6b105c08..20ac1189 100644 --- a/test/fixtures/exports-field/node_modules/exports-field/package.json +++ b/test/fixtures/exports-field/node_modules/exports-field/package.json @@ -9,7 +9,8 @@ "exports": { ".": "./x.js", "./dist/": { - "webpack": ["./lib/lib2/", "./lib/"], + "custom": ["./lib/lib2/", "./lib/"], + "webpack": ["./lib/", "./lib/lib2/"], "node": "./lib/", "default": "./lib/" }, diff --git a/test/fixtures/exports-field2/node_modules/exports-field/package.json b/test/fixtures/exports-field2/node_modules/exports-field/package.json index e6f12074..fd600879 100644 --- a/test/fixtures/exports-field2/node_modules/exports-field/package.json +++ b/test/fixtures/exports-field2/node_modules/exports-field/package.json @@ -9,7 +9,7 @@ "exports": { ".": "./index.js", "./dist/": { - "webpack": ["./lib/lib2/", "./lib/"], + "webpack": ["./lib/", "./lib/lib2/"], "node": "./lib/", "default": "./lib/" } diff --git a/test/fixtures/imports-field-different/package.json b/test/fixtures/imports-field-different/package.json index 3627277e..946d59dd 100644 --- a/test/fixtures/imports-field-different/package.json +++ b/test/fixtures/imports-field-different/package.json @@ -33,7 +33,10 @@ "#dep/array3": ["./a.js"], "#dep/empty": "", "#dep/with-bad": ["../foo", "./a.js"], - "#dep/with-bad2": ["./a.js" ,"../foo"], - "#timezones/": "./data/timezones" + "#dep/with-bad2": ["./a.js", "../foo"], + "#timezones/": "./data/timezones", + "#dep/multi": ["../../test", "./a.js"], + "#dep/multi1": ["../../test", "../../test/foo"], + "#dep/multi2": ["../../test"] } } diff --git a/test/fixtures/imports-field/node_modules/a/package.json b/test/fixtures/imports-field/node_modules/a/package.json index 8d8d3a46..78edd359 100644 --- a/test/fixtures/imports-field/node_modules/a/package.json +++ b/test/fixtures/imports-field/node_modules/a/package.json @@ -9,7 +9,7 @@ "exports": { ".": "./x.js", "./dist/": { - "webpack": ["./lib/lib2/", "./lib/"], + "webpack": ["./lib/", "./lib/lib2/"], "node": "./lib/", "default": "./lib/" }, diff --git a/test/importsField.test.js b/test/importsField.test.js index db19d67e..5f642379 100644 --- a/test/importsField.test.js +++ b/test/importsField.test.js @@ -1192,7 +1192,9 @@ describe("ImportsFieldPlugin", () => { resolver.resolve({}, fixture, "#b", {}, (err, result) => { if (!err) return done(new Error(`expect error, got ${result}`)); expect(err).toBeInstanceOf(Error); - expect(err.message).toMatch(/Trying to access out of package scope/); + expect(err.message).toMatch( + /Invalid "imports" target "\.\.\/b\.js" defined for "#b"/ + ); done(); }); }); @@ -1236,7 +1238,7 @@ describe("ImportsFieldPlugin", () => { if (err) return done(err); if (!result) return done(new Error("No result")); expect(result).toEqual( - path.resolve(fixture, "node_modules/a/lib/lib2/main.js") + path.resolve(fixture, "node_modules/a/lib/main.js") ); done(); }); @@ -1401,9 +1403,9 @@ describe("ImportsFieldPlugin", () => { it("should work with invalid imports #6", done => { resolver.resolve({}, fixture1, "#dep/pattern/a.js", {}, (err, result) => { - if (err) return done(err); - if (!result) return done(new Error("No result")); - expect(result).toEqual(path.resolve(fixture1, "./a.js")); + if (!err) return done(new Error(`expect error, got ${result}`)); + expect(err).toBeInstanceOf(Error); + expect(err.message).toMatch(/Can't resolve '#dep\/pattern\/a.js' in/); done(); }); }); @@ -1419,9 +1421,9 @@ describe("ImportsFieldPlugin", () => { it("should work with invalid imports #8", done => { resolver.resolve({}, fixture1, "#dep/array2", {}, (err, result) => { - if (err) return done(err); - if (!result) return done(new Error("No result")); - expect(result).toEqual(path.resolve(fixture1, "./a.js")); + if (!err) return done(new Error(`expect error, got ${result}`)); + expect(err).toBeInstanceOf(Error); + expect(err.message).toMatch(/Can't resolve '#dep\/array2' in/); done(); }); }); @@ -1470,4 +1472,57 @@ describe("ImportsFieldPlugin", () => { done(); }); }); + + it("should work with invalid imports #12", done => { + resolver.resolve({}, fixture1, "#dep/multi1", {}, (err, result) => { + if (!err) return done(new Error(`expect error, got ${result}`)); + expect(err).toBeInstanceOf(Error); + expect(err.message).toMatch( + /Invalid "imports" target "\.\.\/\.\.\/test\/foo" defined for "#dep\/multi1"/ + ); + done(); + }); + }); + + it("should work with invalid imports #13", done => { + resolver.resolve({}, fixture1, "#dep/multi2", {}, (err, result) => { + if (!err) return done(new Error(`expect error, got ${result}`)); + expect(err).toBeInstanceOf(Error); + expect(err.message).toMatch( + /Invalid "imports" target "\.\.\/\.\.\/test" defined for "#dep\/multi2"/ + ); + done(); + }); + }); + + it("should work with invalid imports #13", done => { + resolver.resolve({}, fixture1, "#dep/multi1", {}, (err, result) => { + if (!err) return done(new Error(`expect error, got ${result}`)); + expect(err).toBeInstanceOf(Error); + expect(err.message).toMatch( + /Invalid "imports" target "\.\.\/\.\.\/test\/foo" defined for "#dep\/multi1"/ + ); + done(); + }); + }); + + it("should work with invalid imports #14", done => { + resolver.resolve({}, fixture1, "#dep/multi2", {}, (err, result) => { + if (!err) return done(new Error(`expect error, got ${result}`)); + expect(err).toBeInstanceOf(Error); + expect(err.message).toMatch( + /Invalid "imports" target "\.\.\/\.\.\/test" defined for "#dep\/multi2"/ + ); + done(); + }); + }); + + it("should work and resolve with array imports", done => { + resolver.resolve({}, fixture1, "#dep/multi", {}, (err, result) => { + if (err) return done(err); + if (!result) return done(new Error("No result")); + expect(result).toEqual(path.resolve(fixture1, "./a.js")); + done(); + }); + }); }); From cea5ad6eb2640ed94fc7f2bb90105472838fcb1a Mon Sep 17 00:00:00 2001 From: "alexander.akait" Date: Fri, 19 Jul 2024 02:31:42 +0300 Subject: [PATCH 2/2] fix: types --- types.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types.d.ts b/types.d.ts index d3bf8bb6..fc09289d 100644 --- a/types.d.ts +++ b/types.d.ts @@ -1108,7 +1108,7 @@ declare namespace exports { export const forEachBail: ( array: T[], iterator: Iterator, - callback: (err?: null | Error, result?: null | Z) => void + callback: (err?: null | Error, result?: null | Z, i?: number) => void ) => void; export type ResolveCallback = ( err: null | ErrorWithDetail,