Skip to content

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

Closed
aduh95 wants to merge 1 commit into
nodejs:v18.x-stagingfrom
aduh95:fix-legacyMainResolve-assert
Closed

[v18.x] esm: fix broken assertion in legacyMainResolve#55708
aduh95 wants to merge 1 commit into
nodejs:v18.x-stagingfrom
aduh95:fix-legacyMainResolve-assert

Conversation

@aduh95

@aduh95aduh95 commented Nov 3, 2024

Copy link
Copy Markdown
Contributor

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

nodejs-github-bot commented Nov 3, 2024

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/loaders

@nodejs-github-botnodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. v18.x labels Nov 3, 2024
@aduh95aduh95 changed the title esm: fix broken assertion in legacyMainResolve[v18.x] esm: fix broken assertion in legacyMainResolveNov 3, 2024
@avivkelleravivkeller 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-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 3, 2024
@nodejs-github-bot

nodejs-github-bot commented Nov 3, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Nov 4, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Nov 5, 2024

Copy link
Copy Markdown
Collaborator

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

@richardlaurichardlau mentioned this pull request Nov 5, 2024
Closed
@nodejs-github-bot

nodejs-github-bot commented Nov 5, 2024

Copy link
Copy Markdown
Collaborator

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

@aduh95
aduh95force-pushed the fix-legacyMainResolve-assert branch from c78ec0c to 0fd6321CompareNovember 5, 2024 10:19
@nodejs-github-bot

nodejs-github-bot commented Nov 5, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Nov 6, 2024

Copy link
Copy Markdown
Collaborator

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

@aduh95
aduh95force-pushed the fix-legacyMainResolve-assert branch from 0fd6321 to 2f519b4CompareNovember 7, 2024 10:02
@nodejs-github-bot

nodejs-github-bot commented Nov 7, 2024

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Nov 7, 2024

Copy link
Copy Markdown
Collaborator

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

@aduh95
aduh95force-pushed the fix-legacyMainResolve-assert branch from 2f519b4 to ba1afa8CompareNovember 7, 2024 12:23
@aduh95
aduh95force-pushed the fix-legacyMainResolve-assert branch from ba1afa8 to f6b7c51CompareNovember 7, 2024 12:24
@nodejs-github-bot

nodejs-github-bot commented Nov 7, 2024

Copy link
Copy Markdown
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: #55708Fixes: #55692
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
@aduh95

aduh95 commented Nov 7, 2024

Copy link
Copy Markdown
ContributorAuthor

Landed in 3059262

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

Reviewers

@richardlaurichardlaurichardlau approved these changes
@marco-ippolitomarco-ippolitomarco-ippolito approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.esmIssues and PRs related to the ECMAScript Modules implementation.needs-ciPRs that need a full CI run.

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