node icon indicating copy to clipboard operation
node copied to clipboard

[v22.x backport] esm: implement import.meta.main

Open Lordfirespeed opened this issue 6 months ago • 5 comments

Backport #57804 to v22.x release line

Lordfirespeed avatar Jun 12 '25 13:06 Lordfirespeed

Review requested:

  • [ ] @nodejs/loaders

nodejs-github-bot avatar Jun 12 '25 13:06 nodejs-github-bot

Should include https://github.com/nodejs/node/pull/58661 if it doesn't already

alexsch01 avatar Jun 12 '25 15:06 alexsch01

We should only pick the commits that have landed on main, this PR have 47 commits that are not on main

aduh95 avatar Jun 12 '25 22:06 aduh95

Ah, my bad, I probably picked commits from the wrong branch.

Lordfirespeed avatar Jun 13 '25 01:06 Lordfirespeed

@aduh95 to adhere to the 'only commits that landed' rule, should I squash the new commits into a landed commit? or are commits for manual resolution after cherry-picking OK?

Lordfirespeed avatar Jun 13 '25 21:06 Lordfirespeed

What's the deal here? I don't understand why CI is failing 🤔

Lordfirespeed avatar Jun 19 '25 14:06 Lordfirespeed

@aduh95 to adhere to the 'only commits that landed' rule, should I squash the new commits into a landed commit? or are commits for manual resolution after cherry-picking OK?

You can use git commit --fixup <commit-sha>, so it's easier to review and squash those additional commits upon landing.

What's the deal here? I don't understand why CI is failing 🤔

It looks like the Coverage CI is broken on that branch, unrelated to your changes. The macOS one timed out, also probably unrelated to your changes.

aduh95 avatar Jun 20 '25 07:06 aduh95

can you include https://github.com/nodejs/node/commit/bba07d7e1edace182894b517a903cb55fb4d5b2b in the backport (if lands cleanly otherwise I can open a backport)

marco-ippolito avatar Jun 25 '25 05:06 marco-ippolito

can you include https://github.com/nodejs/node/commit/bba07d7e1edace182894b517a903cb55fb4d5b2b in the backport (if lands cleanly otherwise I can open a backport)

'In addition to' or 'instead of' d8d3b07 (which is currently included) ?

Lordfirespeed avatar Jun 25 '25 06:06 Lordfirespeed

Sorry I didnt notice it was already included. Looks good

marco-ippolito avatar Jun 25 '25 07:06 marco-ippolito

What's the deal here? I don't understand why CI is failing 🤔

It looks like the Coverage CI is broken on that branch, unrelated to your changes.

FWIW the broken Coverage Windows GitHub Workflow is the same breakage as https://github.com/nodejs/node/issues/58801 (and unrelated to this PR). cc FYI @nodejs/releasers

richardlau avatar Jun 27 '25 13:06 richardlau

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

nodejs-github-bot avatar Jul 21 '25 15:07 nodejs-github-bot

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

nodejs-github-bot avatar Jul 21 '25 22:07 nodejs-github-bot

Rebased onto v22.x-staging

Lordfirespeed avatar Jul 24 '25 00:07 Lordfirespeed

Can you please rebase?

aduh95 avatar Jul 24 '25 18:07 aduh95

Something weird happened and I had to drop a couple of commits, then redo the rebase - let me know whether or not things look OK

Lordfirespeed avatar Jul 24 '25 21:07 Lordfirespeed

Rebased on top of v22.x, fixed the commit message

aduh95 avatar Jul 27 '25 21:07 aduh95

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

nodejs-github-bot avatar Jul 27 '25 21:07 nodejs-github-bot

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

nodejs-github-bot avatar Jul 28 '25 06:07 nodejs-github-bot

Landed in f99aa748c090...2fc89892abaa

aduh95 avatar Jul 28 '25 06:07 aduh95