FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

esm: rename error code related to import attributes by aduh95 · Pull Request #50181 · nodejs/node · GitHub

/ node Public

esm: rename error code related to import attributes - #50181

Merged
nodejs-github-bot merged 3 commits into
nodejs:mainfrom
aduh95:esm-import-attributes
Oct 18, 2023
ConversationCommits (3)ChecksFiles changed
Merged

esm: rename error code related to import attributes#50181
nodejs-github-bot merged 3 commits into
nodejs:mainfrom
aduh95:esm-import-attributes

Conversation

aduh95 commented Oct 14, 2023

Copy link
Copy Markdown
Contributor

No description provided.

aduh95 added experimental Issues and PRs related to experimental features. esm Issues and PRs related to the ECMAScript Modules implementation. needs-citgm PRs that need a CITGM CI run. labels Oct 14, 2023

nodejs-github-bot commented Oct 14, 2023

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/loaders

nodejs-github-bot added errors Issues and PRs related to JavaScript errors originated in Node.js core. needs-ci PRs that need a full CI run. labels Oct 14, 2023
JakobJingleheimer reviewed Oct 14, 2023
View reviewed changes
Comment thread doc/api/errors.md
removed: REPLACEME
-->

An import assertion has failed, preventing the specified module to be imported.

JakobJingleheimer Oct 14, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
Suggested change
An import assertion has failed, preventing the specified module to be imported.
An import assertion has failed, preventing the specified module being imported.

aduh95 Oct 14, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'd prefer to keep it as is, it's the wording used today in https://nodejs.org/api/errors.html#err_import_assertion_type_failed.

Comment thread doc/api/errors.md
removed: REPLACEME
-->

An import assertion is missing, preventing the specified module to be imported.

JakobJingleheimer Oct 14, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
Suggested change
An import assertion is missing, preventing the specified module to be imported.
An import assertion is missing, preventing the specified module being imported.

aduh95 commented Oct 16, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/3333/

Test failures seems to match the ones happening on 21.0.0 release proposal, the only addition is torrent-stream but that's a timeout.

aduh95 added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Oct 16, 2023
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 16, 2023

nodejs-github-bot commented Oct 16, 2023

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Oct 17, 2023

Copy link
Copy Markdown
Collaborator

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

aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. dont-land-on-v18.x labels Oct 18, 2023
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 18, 2023
nodejs-github-bot merged commit 37d4f08 into nodejs:main Oct 18, 2023

nodejs-github-bot commented Oct 18, 2023

Copy link
Copy Markdown
Collaborator

Landed in 37d4f08

targos pushed a commit that referenced this pull request Oct 23, 2023
esm: rename error code related to import attributes
7740bf8 
PR-URL: #50181
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
targos mentioned this pull request Oct 23, 2023
Merged
aduh95 deleted the esm-import-attributes branch October 24, 2023 10:19

targos commented Dec 13, 2023

Copy link
Copy Markdown
Member

Why is this dont-land-on-v20.x ? I need it for #50703

This was referenced Dec 13, 2023
Merged
Closed
targos pushed a commit to targos/node that referenced this pull request Dec 13, 2023
esm: rename error code related to import attributes
20efdc9 
PR-URL: nodejs#50181
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>

aduh95 commented Dec 13, 2023

Copy link
Copy Markdown
Contributor Author

I'm pretty sure I added the dont-land with the assumption that import attributes would not be backported.

aduh95 commented Dec 13, 2023

Copy link
Copy Markdown
Contributor Author

Ah no, I remember now, this PR changes the error codes, and that could break existing code.

targos commented Dec 14, 2023

Copy link
Copy Markdown
Member

Aren't we allowed to break code since this is an experimental feature?

targos commented Dec 14, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

The thing is that a side-effect of #50703 is to change the error code for existing tests (that are updated by this PR).

Forget it. We can keep this out of LTS.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

JakobJingleheimer JakobJingleheimer left review comments
GeoffreyBooth GeoffreyBooth approved these changes
himself65 himself65 approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. errors Issues and PRs related to JavaScript errors originated in Node.js core. esm Issues and PRs related to the ECMAScript Modules implementation. experimental Issues and PRs related to experimental features. needs-ci PRs that need a full CI run. needs-citgm PRs that need a CITGM CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL