Skip to content

esm: better package.json parser errors #35117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 11, 2020
Merged

esm: better package.json parser errors #35117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
esm: better package.json parser errors 
PR-URL: #35117
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
  • Loading branch information
@guybedford @Trott
guybedford authored and Trott committed Sep 11, 2020
commit 3fb7fcd821a9aea27a19ef5f7ffcf5120acb859a
2 changes: 1 addition & 1 deletion lib/internal/errors.js
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
syntax error