From 974e6c0943e763372ec87814f35e471468135bb8 Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Fri, 17 Apr 2020 17:13:16 -0400 Subject: [PATCH 1/9] module: improve error for invalid package targets For targets that are strings that do not start with `./` or `/` the error will now have additional information about what the programming error is. Closes: https://github.com/nodejs/node/issues/32034 --- lib/internal/errors.js | 16 +++++++++++++--- test/es-module/test-esm-exports.mjs | 4 ++++ .../node_modules/pkgexports/package.json | 1 + 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 4c583981725b3b..78cbae7af28b97 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1104,18 +1104,28 @@ E('ERR_INVALID_PACKAGE_CONFIG', (path, message, hasMessage = true) => { }, Error); E('ERR_INVALID_PACKAGE_TARGET', (pkgPath, key, subpath, target, base = undefined) => { + const relError = typeof target === 'string' + && target.length && !target.startsWith('./'); if (key === null) { if (subpath !== '') { return `Invalid "exports" target ${JSONStringify(target)} defined ` + `for '${subpath}' in the package config ${pkgPath} imported from ` + - base; + `${base}.${relError ? ' - targets must start with "./"' : ''}`; } else { return `Invalid "exports" main target ${target} defined in the ` + - `package config ${pkgPath} imported from ${base}.`; + `package config ${pkgPath} imported from ${base}.${relError ? + ' - targets must start with "./"' : ''}`; } } else if (key === '.') { return `Invalid "exports" main target ${JSONStringify(target)} defined ` + - `in the package config ${pkgPath}${sep}package.json`; + `in the package config ${pkgPath}${sep}package.json${relError ? + ' - targets must start with "./"' : ''}`; + } else if (typeof target === 'string' && target !== '' && + !target.startsWith('./')) { + return `Invalid "exports" target ${JSONStringify(target)} defined for '${ + StringPrototypeSlice(key, 0, -subpath.length || key.length)}' in the ` + + `package config ${pkgPath}${sep}package.json. ` + + '- targets must start with `./`'; } else { return `Invalid "exports" target ${JSONStringify(target)} defined for '${ StringPrototypeSlice(key, 0, -subpath.length || key.length)}' in the ` + diff --git a/test/es-module/test-esm-exports.mjs b/test/es-module/test-esm-exports.mjs index f71e2172951843..2b58f28344522c 100644 --- a/test/es-module/test-esm-exports.mjs +++ b/test/es-module/test-esm-exports.mjs @@ -78,6 +78,7 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js'; ['pkgexports/null', './null'], ['pkgexports/invalid2', './invalid2'], ['pkgexports/invalid3', './invalid3'], + ['pkgexports/invalid5', 'invalid5'], // Missing / invalid fallbacks ['pkgexports/nofallback1', './nofallback1'], ['pkgexports/nofallback2', './nofallback2'], @@ -106,6 +107,9 @@ import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js'; strictEqual(err.code, 'ERR_INVALID_PACKAGE_TARGET'); assertStartsWith(err.message, 'Invalid "exports"'); assertIncludes(err.message, subpath); + if (!subpath.startsWith('./')) { + assertIncludes(err.message, 'targets must start with'); + } })); } diff --git a/test/fixtures/node_modules/pkgexports/package.json b/test/fixtures/node_modules/pkgexports/package.json index f3ec20c49b2b91..43ccf7795b978e 100644 --- a/test/fixtures/node_modules/pkgexports/package.json +++ b/test/fixtures/node_modules/pkgexports/package.json @@ -13,6 +13,7 @@ "./invalid2": 1234, "./invalid3": "", "./invalid4": {}, + "./invalid5": "invalid5.js", "./fallbackdir/": [[], null, {}, "builtin:x/", "./fallbackfile", "./"], "./fallbackfile": [[], null, {}, "builtin:x", "./asdf.js"], "./nofallback1": [], From c55afbaebb980400df546cf8a67bdf6d8a9ee2df Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Fri, 17 Apr 2020 17:25:19 -0400 Subject: [PATCH 2/9] fixup: lint issues --- lib/internal/errors.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 78cbae7af28b97..858bea805205c0 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1104,8 +1104,8 @@ E('ERR_INVALID_PACKAGE_CONFIG', (path, message, hasMessage = true) => { }, Error); E('ERR_INVALID_PACKAGE_TARGET', (pkgPath, key, subpath, target, base = undefined) => { - const relError = typeof target === 'string' - && target.length && !target.startsWith('./'); + const relError = typeof target === 'string' && + target.length && !target.startsWith('./'); if (key === null) { if (subpath !== '') { return `Invalid "exports" target ${JSONStringify(target)} defined ` + @@ -1114,18 +1114,18 @@ E('ERR_INVALID_PACKAGE_TARGET', } else { return `Invalid "exports" main target ${target} defined in the ` + `package config ${pkgPath} imported from ${base}.${relError ? - ' - targets must start with "./"' : ''}`; + ' - targets must start with "./"' : ''}`; } } else if (key === '.') { return `Invalid "exports" main target ${JSONStringify(target)} defined ` + `in the package config ${pkgPath}${sep}package.json${relError ? - ' - targets must start with "./"' : ''}`; + ' - targets must start with "./"' : ''}`; } else if (typeof target === 'string' && target !== '' && !target.startsWith('./')) { - return `Invalid "exports" target ${JSONStringify(target)} defined for '${ - StringPrototypeSlice(key, 0, -subpath.length || key.length)}' in the ` + - `package config ${pkgPath}${sep}package.json. ` + - '- targets must start with `./`'; + return `Invalid "exports" target ${JSONStringify(target)} defined for '${ + StringPrototypeSlice(key, 0, -subpath.length || key.length)}' in the ` + + `package config ${pkgPath}${sep}package.json. ` + + '- targets must start with `./`'; } else { return `Invalid "exports" target ${JSONStringify(target)} defined for '${ StringPrototypeSlice(key, 0, -subpath.length || key.length)}' in the ` + From 91e8e80c9261982a140752e6ba821f80b5451a9c Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Fri, 17 Apr 2020 17:28:19 -0400 Subject: [PATCH 3/9] fixup: primordials --- lib/internal/errors.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 858bea805205c0..d77f09d3c19fdf 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -20,6 +20,7 @@ const { ObjectDefineProperty, ObjectKeys, StringPrototypeSlice, + StringPrototypeStartsWith, Symbol, SymbolFor, WeakMap, @@ -1105,7 +1106,7 @@ E('ERR_INVALID_PACKAGE_CONFIG', (path, message, hasMessage = true) => { E('ERR_INVALID_PACKAGE_TARGET', (pkgPath, key, subpath, target, base = undefined) => { const relError = typeof target === 'string' && - target.length && !target.startsWith('./'); + target.length && !StringPrototypeStartsWith(target, './'); if (key === null) { if (subpath !== '') { return `Invalid "exports" target ${JSONStringify(target)} defined ` + @@ -1121,7 +1122,7 @@ E('ERR_INVALID_PACKAGE_TARGET', `in the package config ${pkgPath}${sep}package.json${relError ? ' - targets must start with "./"' : ''}`; } else if (typeof target === 'string' && target !== '' && - !target.startsWith('./')) { + !StringPrototypeStartsWith(target, './')) { return `Invalid "exports" target ${JSONStringify(target)} defined for '${ StringPrototypeSlice(key, 0, -subpath.length || key.length)}' in the ` + `package config ${pkgPath}${sep}package.json. ` + From bb4a755d67b067d853afeb6788cb63bb0c1dc43e Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Mon, 20 Apr 2020 22:07:39 -0400 Subject: [PATCH 4/9] Update lib/internal/errors.js Co-Authored-By: Guy Bedford --- lib/internal/errors.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index d77f09d3c19fdf..7edcc1c49fb85b 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1114,7 +1114,7 @@ E('ERR_INVALID_PACKAGE_TARGET', `${base}.${relError ? ' - targets must start with "./"' : ''}`; } else { return `Invalid "exports" main target ${target} defined in the ` + - `package config ${pkgPath} imported from ${base}.${relError ? + `package config ${pkgPath} imported from ${base}${relError ? ' - targets must start with "./"' : ''}`; } } else if (key === '.') { From 5ca9689c64e989e15bb04957ca494f4b7c34169c Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Mon, 20 Apr 2020 22:07:46 -0400 Subject: [PATCH 5/9] Update lib/internal/errors.js Co-Authored-By: Guy Bedford --- lib/internal/errors.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 7edcc1c49fb85b..d059c7d985cf2b 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1126,7 +1126,7 @@ E('ERR_INVALID_PACKAGE_TARGET', return `Invalid "exports" target ${JSONStringify(target)} defined for '${ StringPrototypeSlice(key, 0, -subpath.length || key.length)}' in the ` + `package config ${pkgPath}${sep}package.json. ` + - '- targets must start with `./`'; + '- targets must start with "./"'; } else { return `Invalid "exports" target ${JSONStringify(target)} defined for '${ StringPrototypeSlice(key, 0, -subpath.length || key.length)}' in the ` + From c5f08ce4245db24a30e20c2a14744f828bf41d75 Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Tue, 21 Apr 2020 15:38:26 -0400 Subject: [PATCH 6/9] Update lib/internal/errors.js Co-Authored-By: Geoffrey Booth --- lib/internal/errors.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index d059c7d985cf2b..5e017acecd9a16 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1125,8 +1125,8 @@ E('ERR_INVALID_PACKAGE_TARGET', !StringPrototypeStartsWith(target, './')) { return `Invalid "exports" target ${JSONStringify(target)} defined for '${ StringPrototypeSlice(key, 0, -subpath.length || key.length)}' in the ` + - `package config ${pkgPath}${sep}package.json. ` + - '- targets must start with "./"'; + `package config ${pkgPath}${sep}package.json; ` + + 'targets must start with "./"'; } else { return `Invalid "exports" target ${JSONStringify(target)} defined for '${ StringPrototypeSlice(key, 0, -subpath.length || key.length)}' in the ` + From 51de45b9a16b714536448e38e507b0e51b64a1b7 Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Tue, 21 Apr 2020 15:38:33 -0400 Subject: [PATCH 7/9] Update lib/internal/errors.js Co-Authored-By: Geoffrey Booth --- lib/internal/errors.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 5e017acecd9a16..73d3597fc2334d 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1115,7 +1115,7 @@ E('ERR_INVALID_PACKAGE_TARGET', } else { return `Invalid "exports" main target ${target} defined in the ` + `package config ${pkgPath} imported from ${base}${relError ? - ' - targets must start with "./"' : ''}`; + '; targets must start with "./"' : ''}`; } } else if (key === '.') { return `Invalid "exports" main target ${JSONStringify(target)} defined ` + From 8e04ec1866c464c920537d12943b94b25efdf01f Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Tue, 21 Apr 2020 15:38:42 -0400 Subject: [PATCH 8/9] Update lib/internal/errors.js Co-Authored-By: Geoffrey Booth --- lib/internal/errors.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 73d3597fc2334d..1120990bb4306b 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1120,7 +1120,7 @@ E('ERR_INVALID_PACKAGE_TARGET', } else if (key === '.') { return `Invalid "exports" main target ${JSONStringify(target)} defined ` + `in the package config ${pkgPath}${sep}package.json${relError ? - ' - targets must start with "./"' : ''}`; + '; targets must start with "./"' : ''}`; } else if (typeof target === 'string' && target !== '' && !StringPrototypeStartsWith(target, './')) { return `Invalid "exports" target ${JSONStringify(target)} defined for '${ From ef6c799254dadaeb23c4b1b0b96353c62f52cb16 Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Tue, 21 Apr 2020 15:38:50 -0400 Subject: [PATCH 9/9] Update lib/internal/errors.js Co-Authored-By: Geoffrey Booth --- lib/internal/errors.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 1120990bb4306b..fa009b90465cb4 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1111,7 +1111,7 @@ E('ERR_INVALID_PACKAGE_TARGET', if (subpath !== '') { return `Invalid "exports" target ${JSONStringify(target)} defined ` + `for '${subpath}' in the package config ${pkgPath} imported from ` + - `${base}.${relError ? ' - targets must start with "./"' : ''}`; + `${base}.${relError ? '; targets must start with "./"' : ''}`; } else { return `Invalid "exports" main target ${target} defined in the ` + `package config ${pkgPath} imported from ${base}${relError ?