Skip to content
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

Jump to bottom

esm: better package.json parser errors #35117

Merged
merged 1 commit into from
Sep 11, 2020
Merged

esm: better package.json parser errors #35117

merged 1 commit into from
Sep 11, 2020

Conversation

guybedford
Copy link
Contributor

This is a follow-up to #35072 to add more context to package.json parse errors.

Specifically the package / module being imported or imported from when the package.json parser error was encountered.

These kinds of cases make up for the fact that ESM, unlike require, does not have a require stack to see what was the import in the first case that caused the issue.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added errors Issues and PRs related to JavaScript errors originated in Node.js core. esm Issues and PRs related to the ECMAScript Modules implementation. labels Sep 9, 2020
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/modules

@guybedford guybedford mentioned this pull request Sep 9, 2020
Closed
2 tasks
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/33113/

mhdawson
mhdawson approved these changes Sep 9, 2020
View reviewed changes
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, assuming since modules is still experimental there is no concern about changing the error message.

@Trott
Copy link
Member

Trott commented Sep 11, 2020

LGTM, assuming since modules is still experimental there is no concern about changing the error message.

IIUC, since this changes the message property only and not the code property, it should be semver-patch no matter what the status of ESM is.

@Trott Trott merged commit 3fb7fcd into nodejs:master Sep 11, 2020
@Trott
Copy link
Member

Trott commented Sep 11, 2020

Landed in 3fb7fcd

@guybedford @Trott
esm: better package.json parser errors
3fb7fcd 
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>
ruyadorno pushed a commit that referenced this pull request Sep 17, 2020
@guybedford @ruyadorno
esm: better package.json parser errors
6e622d6 
PR-URL: #35117
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@ruyadorno ruyadorno mentioned this pull request Sep 21, 2020
Merged
4 tasks
guybedford added a commit to guybedford/node that referenced this pull request Sep 28, 2020
@guybedford
esm: better package.json parser errors
b29b966 
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>
codebytere pushed a commit that referenced this pull request Oct 1, 2020
@guybedford @codebytere
esm: better package.json parser errors
bf7b796 
PR-URL: #35117
Backport-PR-URL: #35385
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@codebytere codebytere mentioned this pull request Oct 4, 2020
Merged
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
@guybedford @joesepi
esm: better package.json parser errors
bfd036d 
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Trott Trott Trott approved these changes

@addaleax addaleax addaleax approved these changes

@mhdawson mhdawson mhdawson approved these changes

Assignees
No one assigned
Labels
errors Issues and PRs related to JavaScript errors originated in Node.js core. esm Issues and PRs related to the ECMAScript Modules implementation.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@guybedford @nodejs-github-bot @Trott @addaleax @mhdawson