Skip to content

Commit

Permalink
fix: exports and imports array target resolving
Browse files Browse the repository at this point in the history
  • Loading branch information
@alexander-akait
alexander-akait committed Jul 18, 2024
1 parent ce50aa8 commit fd86859
Show file tree
Hide file tree
Showing 12 changed files with 294 additions and 47 deletions.
16 changes: 11 additions & 5 deletions lib/ExportsFieldPlugin.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,17 +125,18 @@ 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();

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 "./"`
Expand All @@ -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 "./"`
)
);
}
Expand All @@ -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);
}
);
},
/**
Expand Down
28 changes: 22 additions & 6 deletions lib/ImportsFieldPlugin.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand All @@ -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();
Expand All @@ -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 "./"`
)
);
}
Expand All @@ -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;
}
Expand All @@ -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);
}
);
}
}
Expand Down
4 changes: 2 additions & 2 deletions lib/forEachBail.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
* @template Z
* @param {T[]} array array
* @param {Iterator<T, Z>} 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) {
Expand All @@ -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;
Expand Down
3 changes: 0 additions & 3 deletions test/__snapshots__/exportsField.test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
8 changes: 0 additions & 8 deletions test/__snapshots__/importsField.test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
171 changes: 162 additions & 9 deletions test/exportsField.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
});
Expand All @@ -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(
Expand Down Expand Up @@ -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();
}
Expand All @@ -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();
}
Expand Down Expand Up @@ -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();
}
);
Expand All @@ -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();
}
);
Expand Down Expand Up @@ -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();
});
});
Expand Down Expand Up @@ -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();
}
);
Expand Down Expand Up @@ -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();
}
);
});
});
Loading

0 comments on commit fd86859

Please sign in to comment.