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

[v18.x] esm: fix broken assertion in legacyMainResolve #55708

Closed
wants to merge 1 commit into from
Closed

[v18.x] esm: fix broken assertion in legacyMainResolve #55708

wants to merge 1 commit into from

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Nov 3, 2024

On main, this was superseded by #48325, but in #49644 (comment) it was decided to not backport it to v18.x. Unfortunately we have a lack of coverage for this part of the code, and we don't have a repro for the issue, so this PR doesn't contain tests, it'd be good to have one though.

Fixes: #55692

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/loaders

@nodejs-github-bot nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. v18.x Issues that can be reproduced on v18.x or PRs targeting the v18.x-staging branch. labels Nov 3, 2024
@aduh95 aduh95 changed the title esm: fix broken assertion in legacyMainResolve [v18.x] esm: fix broken assertion in legacyMainResolve Nov 3, 2024
@avivkeller avivkeller added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Nov 3, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 3, 2024
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@richardlau richardlau mentioned this pull request Nov 5, 2024
Closed
@nodejs-github-bot
Copy link
Collaborator

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

@aduh95 aduh95 force-pushed the fix-legacyMainResolve-assert branch from c78ec0c to 0fd6321 Compare November 5, 2024 10:19
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@aduh95 aduh95 force-pushed the fix-legacyMainResolve-assert branch from 0fd6321 to 2f519b4 Compare November 7, 2024 10:02
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@aduh95 aduh95 force-pushed the fix-legacyMainResolve-assert branch from 2f519b4 to ba1afa8 Compare November 7, 2024 12:23
@aduh95 aduh95 force-pushed the fix-legacyMainResolve-assert branch from ba1afa8 to f6b7c51 Compare November 7, 2024 12:24
@nodejs-github-bot
Copy link
Collaborator

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

aduh95 added a commit that referenced this pull request Nov 7, 2024
@aduh95
esm: fix broken assertion in legacyMainResolve
3059262 
PR-URL: #55708
Fixes: #55692
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
@aduh95
Copy link
Contributor Author

aduh95 commented Nov 7, 2024

Landed in 3059262

@aduh95 aduh95 closed this Nov 7, 2024
@aduh95 aduh95 deleted the fix-legacyMainResolve-assert branch November 7, 2024 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardlau richardlau richardlau approved these changes

@marco-ippolito marco-ippolito marco-ippolito approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. v18.x Issues that can be reproduced on v18.x or PRs targeting the v18.x-staging branch.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@aduh95 @nodejs-github-bot @richardlau @marco-ippolito @avivkeller