GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — esm: refactor `globalPreload` tests by GeoffreyBooth · Pull Request #49493 · nodejs/node

Conversation

@GeoffreyBooth

@GeoffreyBooth GeoffreyBooth commented Sep 4, 2023

Copy link
Copy Markdown
Member

Similar to #49465 and supporting #49144, this PR refactors two tests that currently use the module customization hook globalPreload to use its replacement register / initialize instead. This spins off work from #49144 into a standalone PR so that we have examples to direct people to to show how they can do similar refactors. In that vein, after discussion with @aduh95 we would like to land this PR as two distinct commits, so that we can share either the link to this PR (to cite both tests) or the links to the two commits to reference one refactored test or the other. @nodejs/loaders

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss reacted with eyes emoji
All reactions
  • 👀 1 reaction
@GeoffreyBooth GeoffreyBooth added test Issues and PRs related to Node.js core tests and test infrastructure. esm Issues and PRs related to the ECMAScript Modules implementation. loaders Issues and PRs related to ES module loaders. labels Sep 4, 2023
@GeoffreyBooth
GeoffreyBooth requested review from JakobJingleheimer and aduh95 September 4, 2023 22:28
@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Sep 4, 2023
@GeoffreyBooth
GeoffreyBooth force-pushed the refactor-globalpreload-tests branch from 58636f2 to dcecdcf Compare September 4, 2023 22:34
aduh95
aduh95 reviewed Sep 5, 2023
View reviewed changes
Comment thread test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs Resolved

Uh oh!

There was an error while loading. Please reload this page.
@GeoffreyBooth
GeoffreyBooth force-pushed the refactor-globalpreload-tests branch 2 times, most recently from 9b0614e to cef85dc Compare September 5, 2023 18:24
JakobJingleheimer
JakobJingleheimer approved these changes Sep 5, 2023
View reviewed changes

@JakobJingleheimer JakobJingleheimer left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

🙌

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/es-module/test-esm-loader-resolve-type.mjs Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread test/fixtures/es-module-loaders/builtin-named-exports.mjs Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread test/fixtures/es-module-loaders/hook-resolve-type-loader.mjs Resolved

Uh oh!

There was an error while loading. Please reload this page.
@GeoffreyBooth
esm: refactor test-esm-named-exports
fa041a2
@GeoffreyBooth
GeoffreyBooth force-pushed the refactor-globalpreload-tests branch from cef85dc to 7b3dfaf Compare September 5, 2023 20:49
@GeoffreyBooth GeoffreyBooth added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Sep 5, 2023
@nodejs-github-bot

nodejs-github-bot commented Sep 5, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/53748/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
aduh95
aduh95 approved these changes Sep 5, 2023
View reviewed changes
aduh95
aduh95 reviewed Sep 5, 2023
View reviewed changes
Comment thread test/es-module/test-esm-loader-resolve-type.mjs Resolved

Uh oh!

There was an error while loading. Please reload this page.
@GeoffreyBooth
esm: refactor test-esm-loader-resolve-type
e5470e0
@GeoffreyBooth
GeoffreyBooth force-pushed the refactor-globalpreload-tests branch from 7b3dfaf to e5470e0 Compare September 5, 2023 20:56
@nodejs-github-bot

nodejs-github-bot commented Sep 5, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/53749/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Sep 5, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/53750/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs nodejs deleted a comment from nodejs-github-bot Sep 5, 2023
aduh95
aduh95 approved these changes Sep 5, 2023
View reviewed changes
@nodejs-github-bot

nodejs-github-bot commented Sep 5, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/53752/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Sep 6, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/53753/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot mentioned this pull request Sep 6, 2023
CI Reliability 2023-09-06 nodejs/reliability#657
Open
25 tasks
@GeoffreyBooth GeoffreyBooth added the commit-queue-rebase PRs the Commit Queue should land as multiple self-contained commits. label Sep 6, 2023
@GeoffreyBooth GeoffreyBooth mentioned this pull request Sep 6, 2023
esm: remove globalPreload hook (superseded by initialize) #49144
Merged
@JakobJingleheimer JakobJingleheimer added commit-queue PRs queued for automated landing through the Commit Queue. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Sep 6, 2023
@nodejs-github-bot

nodejs-github-bot commented Sep 6, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/53758/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Sep 6, 2023
@nodejs-github-bot

nodejs-github-bot commented Sep 6, 2023

Copy link
Copy Markdown
Collaborator

Landed in 6b135a1...63c3846
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot nodejs-github-bot closed this Sep 6, 2023
nodejs-github-bot pushed a commit that referenced this pull request Sep 6, 2023
@GeoffreyBooth @nodejs-github-bot
esm: refactor test-esm-named-exports
a927ade 
PR-URL: #49493
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Sep 6, 2023
@GeoffreyBooth @nodejs-github-bot
esm: refactor test-esm-loader-resolve-type
63c3846 
PR-URL: #49493
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
@GeoffreyBooth
GeoffreyBooth deleted the refactor-globalpreload-tests branch September 6, 2023 22:31
This was referenced Sep 7, 2023
CI Reliability 2023-09-07 nodejs/reliability#658
Open
CI Reliability 2023-09-08 nodejs/reliability#659
Open
CI Reliability 2023-09-09 nodejs/reliability#660
Open
CI Reliability 2023-09-10 nodejs/reliability#661
Open
@JakobJingleheimer JakobJingleheimer mentioned this pull request Sep 13, 2023
ESM Loaders breaking changes notification nodejs/loaders#103
Open
ruyadorno pushed a commit that referenced this pull request Sep 28, 2023
@GeoffreyBooth @ruyadorno
esm: refactor test-esm-named-exports
17823b3 
PR-URL: #49493
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
ruyadorno pushed a commit that referenced this pull request Sep 28, 2023
@GeoffreyBooth @ruyadorno
esm: refactor test-esm-loader-resolve-type
e74a075 
PR-URL: #49493
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
This was referenced Sep 28, 2023
v20.7.1 proposal #49917
Closed
v20.8.0 proposal #49932
Merged
@GeoffreyBooth GeoffreyBooth mentioned this pull request Oct 22, 2023
node 20 segfaults when our loader runs #50280
Closed
targos pushed a commit to targos/node that referenced this pull request Nov 11, 2023
@GeoffreyBooth @targos
esm: refactor test-esm-named-exports
d785f83 
PR-URL: nodejs#49493
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
targos pushed a commit to targos/node that referenced this pull request Nov 11, 2023
@GeoffreyBooth @targos
esm: refactor test-esm-loader-resolve-type
9a7a8c5 
PR-URL: nodejs#49493
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
@targos targos mentioned this pull request Nov 11, 2023
[v18.x] Backport most ESM and customization hook changes #50669
Closed
@targos targos added the backported-to-v18.x label Nov 23, 2023
targos pushed a commit that referenced this pull request Nov 23, 2023
@GeoffreyBooth @targos
esm: refactor test-esm-named-exports
9b7c9d9 
PR-URL: #49493
Backport-PR-URL: #50669
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
targos pushed a commit that referenced this pull request Nov 23, 2023
@GeoffreyBooth @targos
esm: refactor test-esm-loader-resolve-type
0875867 
PR-URL: #49493
Backport-PR-URL: #50669
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
@targos targos mentioned this pull request Nov 28, 2023
v18.19.0 release proposal #50953
Merged
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
@sercher
esm: refactor test-esm-named-exports
64bbde6 
PR-URL: nodejs/node#49493
Backport-PR-URL: nodejs/node#50669
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
@sercher
esm: refactor test-esm-loader-resolve-type
fe729ec 
PR-URL: nodejs/node#49493
Backport-PR-URL: nodejs/node#50669
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
@sercher
esm: refactor test-esm-named-exports
63f4688 
PR-URL: nodejs/node#49493
Backport-PR-URL: nodejs/node#50669
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
@sercher
esm: refactor test-esm-loader-resolve-type
f60231d 
PR-URL: nodejs/node#49493
Backport-PR-URL: nodejs/node#50669
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JakobJingleheimer JakobJingleheimer JakobJingleheimer approved these changes
@aduh95 aduh95 aduh95 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. commit-queue-rebase PRs the Commit Queue should land as multiple self-contained commits. esm Issues and PRs related to the ECMAScript Modules implementation. loaders Issues and PRs related to ES module loaders. needs-ci PRs that need a full CI run. test Issues and PRs related to Node.js core tests and test infrastructure.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@GeoffreyBooth @nodejs-github-bot @JakobJingleheimer @aduh95 @targos
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.