Skip to content

Commit

Permalink
esm: better package.json parser errors
Browse files Browse the repository at this point in the history
PR-URL: nodejs#35117
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
guybedford authored and joesepi committed Oct 22, 2020
1 parent c658026 commit bfd036d
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 14 deletions.
2 changes: 1 addition & 1 deletion lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1124,7 +1124,7 @@ E('ERR_INVALID_OPT_VALUE', (name, value, reason = '') => {
E('ERR_INVALID_OPT_VALUE_ENCODING',
'The value "%s" is invalid for option "encoding"', TypeError);
E('ERR_INVALID_PACKAGE_CONFIG', (path, base, message) => {
return `Invalid package config ${path}${base ? ` imported from ${base}` :
return `Invalid package config ${path}${base ? ` while importing ${base}` :
''}${message ? `. ${message}` : ''}`;
}, Error);
E('ERR_INVALID_PACKAGE_TARGET',
Expand Down
21 changes: 13 additions & 8 deletions lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ function tryStatSync(path) {
}
}

function getPackageConfig(path) {
function getPackageConfig(path, specifier, base) {
const existing = packageJSONCache.get(path);
if (existing !== undefined) {
return existing;
Expand All @@ -106,7 +106,11 @@ function getPackageConfig(path) {
try {
packageJSON = JSONParse(source);
} catch (error) {
throw new ERR_INVALID_PACKAGE_CONFIG(path, null, error.message);
throw new ERR_INVALID_PACKAGE_CONFIG(
path,
(base ? `"${specifier}" from ` : '') + fileURLToPath(base || specifier),
error.message
);
}

let { imports, main, name, type } = packageJSON;
Expand All @@ -130,13 +134,14 @@ function getPackageConfig(path) {
return packageConfig;
}

function getPackageScopeConfig(resolved, base) {
function getPackageScopeConfig(resolved) {
let packageJSONUrl = new URL('./package.json', resolved);
while (true) {
const packageJSONPath = packageJSONUrl.pathname;
if (StringPrototypeEndsWith(packageJSONPath, 'node_modules/package.json'))
break;
const packageConfig = getPackageConfig(fileURLToPath(packageJSONUrl), base);
const packageConfig = getPackageConfig(fileURLToPath(packageJSONUrl),
resolved);
if (packageConfig.exists) return packageConfig;

const lastPackageJSONUrl = packageJSONUrl;
Expand Down Expand Up @@ -497,7 +502,7 @@ function packageImportsResolve(name, base, conditions) {
throw new ERR_INVALID_MODULE_SPECIFIER(name, reason, fileURLToPath(base));
}
let packageJSONUrl;
const packageConfig = getPackageScopeConfig(base, base);
const packageConfig = getPackageScopeConfig(base);
if (packageConfig.exists) {
packageJSONUrl = pathToFileURL(packageConfig.pjsonPath);
const imports = packageConfig.imports;
Expand Down Expand Up @@ -535,7 +540,7 @@ function packageImportsResolve(name, base, conditions) {
}

function getPackageType(url) {
const packageConfig = getPackageScopeConfig(url, url);
const packageConfig = getPackageScopeConfig(url);
return packageConfig.type;
}

Expand Down Expand Up @@ -580,7 +585,7 @@ function packageResolve(specifier, base, conditions) {
StringPrototypeSlice(specifier, separatorIndex));

// ResolveSelf
const packageConfig = getPackageScopeConfig(base, base);
const packageConfig = getPackageScopeConfig(base);
if (packageConfig.exists) {
const packageJSONUrl = pathToFileURL(packageConfig.pjsonPath);
if (packageConfig.name === packageName &&
Expand Down Expand Up @@ -608,7 +613,7 @@ function packageResolve(specifier, base, conditions) {
}

// Package match.
const packageConfig = getPackageConfig(packageJSONPath, base);
const packageConfig = getPackageConfig(packageJSONPath, specifier, base);
if (packageConfig.exports !== undefined && packageConfig.exports !== null)
return packageExportsResolve(
packageJSONUrl, packageSubpath, packageConfig, base, conditions
Expand Down
8 changes: 3 additions & 5 deletions test/es-module/test-esm-invalid-pjson.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,9 @@ child.on('close', mustCall((code, signal) => {
strictEqual(signal, null);
ok(
stderr.includes(
[
'[ERR_INVALID_PACKAGE_CONFIG]: ',
`Invalid package config ${invalidJson}. `,
`Unexpected token } in JSON at position ${isWindows ? 16 : 14}`
].join(''),
`[ERR_INVALID_PACKAGE_CONFIG]: Invalid package config ${invalidJson} ` +
`while importing "invalid-pjson" from ${entry}. ` +
`Unexpected token } in JSON at position ${isWindows ? 16 : 14}`
),
stderr);
}));
1 change: 1 addition & 0 deletions test/fixtures/es-modules/pjson-invalid/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
syntax error

0 comments on commit bfd036d

Please sign in to comment.