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: convert `resolve` hook to synchronous by JakobJingleheimer · Pull Request #43363 · nodejs/node

Conversation

@JakobJingleheimer

@JakobJingleheimer JakobJingleheimer commented Jun 9, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Notable changes

This PR converts the resolve hook from asynchronous to synchronous to align with the general ecosystem (most namely browsers). When a resolve hook returns a thenable (eg async or promise), an error is specifically thrown.

It also converts import.meta.resolve() to synchronous.

Unblocks

  • esm: add chaining to loaders #42623
  • esm: fix http(s) import via custom loader #43130
  • esm: refactor responseURL handling #43164
  • esm: fix chain advances when loader calls next<HookName> multiple times #43303

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@JakobJingleheimer
esm: convert resolve hook to synchronous
37abe52
@JakobJingleheimer JakobJingleheimer added esm Issues and PRs related to the ECMAScript Modules implementation. loaders Issues and PRs related to ES module loaders. labels Jun 9, 2022
@nodejs-github-bot

nodejs-github-bot commented Jun 9, 2022

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/modules
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jun 9, 2022
GeoffreyBooth
GeoffreyBooth reviewed Jun 9, 2022
View reviewed changes
Comment thread lib/internal/modules/esm/resolve.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
aduh95
aduh95 reviewed Jun 10, 2022
View reviewed changes
Comment thread lib/internal/modules/esm/loader.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
aduh95
aduh95 reviewed Jun 10, 2022
View reviewed changes
Comment thread lib/internal/modules/esm/loader.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
aduh95
aduh95 reviewed Jun 10, 2022
View reviewed changes
Comment thread lib/internal/modules/esm/loader.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
aduh95
aduh95 reviewed Jun 10, 2022
View reviewed changes
Comment thread test/es-module/test-esm-import-meta-resolve.mjs Resolved

Uh oh!

There was an error while loading. Please reload this page.
aduh95
aduh95 reviewed Jun 10, 2022
View reviewed changes
Comment thread test/es-module/test-esm-loader-search.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
aduh95 added a commit to aduh95/node that referenced this pull request Jun 11, 2022
@aduh95
test: improve coverage for load hooks
d3789b5 
Refs: nodejs#43363 (comment)
@aduh95 aduh95 mentioned this pull request Jun 11, 2022
test: improve coverage for load hooks #43374
Merged
guybedford
guybedford reviewed Jun 11, 2022
View reviewed changes

@guybedford guybedford left a comment

Copy link
Copy Markdown
Contributor

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.

Would love to see this!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread lib/internal/modules/esm/initialize_import_meta.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
aduh95
aduh95 reviewed Jun 12, 2022
View reviewed changes
Comment thread lib/internal/modules/esm/loader.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
aduh95
aduh95 reviewed Jun 12, 2022
View reviewed changes
Comment thread lib/internal/modules/esm/loader.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
aduh95
aduh95 reviewed Jun 12, 2022
View reviewed changes
Comment thread test/es-module/test-esm-import-meta-resolve.mjs Resolved

Uh oh!

There was an error while loading. Please reload this page.
JakobJingleheimer added 3 commits June 12, 2022 16:01
@JakobJingleheimer
fixup: fix import.meta.resolve
edb6ba5
@JakobJingleheimer
fixup: correct assert.throws()
ae8ef63
@JakobJingleheimer
fixup: support detecting when a resolve hook returns a promise
8bf7965
@JakobJingleheimer
JakobJingleheimer force-pushed the esm/synchronous-resolve-hook branch 2 times, most recently from 8425665 to c3b979c Compare June 12, 2022 18:24
@JakobJingleheimer
fixup: change async/thenable detection
ab935b6
@JakobJingleheimer
JakobJingleheimer force-pushed the esm/synchronous-resolve-hook branch from c3b979c to ab935b6 Compare June 12, 2022 18:38
JakobJingleheimer added 2 commits June 12, 2022 20:44
@JakobJingleheimer
fixup: update ESM doc for resolve hook
28d813f
@JakobJingleheimer
fixup: revert back to arg destructuring
b332b97
@JakobJingleheimer

This comment was marked as resolved.

Sign in to view
@GeoffreyBooth GeoffreyBooth mentioned this pull request Jun 14, 2022
esm: Make import.meta.resolve(…) take an options argument #31861
Closed
@jasnell

jasnell commented Jun 14, 2022

Copy link
Copy Markdown
Member

This is frustrating (due to the whatwg refusing to acknowledge that having this be async is a legitimate case) but ok. We might want to consider keeping an async alternative to this in module.
JakobJingleheimer reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

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 Jun 18, 2022
@nodejs-github-bot
nodejs-github-bot merged commit 90b634a into nodejs:main Jun 18, 2022
@nodejs-github-bot

nodejs-github-bot commented Jun 18, 2022

Copy link
Copy Markdown
Collaborator

Landed in 90b634a
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@GeoffreyBooth
GeoffreyBooth deleted the esm/synchronous-resolve-hook branch June 18, 2022 17:51
@GeoffreyBooth GeoffreyBooth removed needs-ci PRs that need a full CI run. commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. commit-queue-squash PRs the Commit Queue should land as one squashed commit. labels Jun 18, 2022
@github-actions github-actions Bot mentioned this pull request Jun 19, 2022
CI Reliability 2022-06-19 nodejs/reliability#305
Open
30 tasks
This was referenced Jun 19, 2022
Node.js Loaders Team Meeting 2022-06-21 nodejs/loaders#88
Closed
proposal: preImport nodejs/loaders#89
Closed
This was referenced Jun 20, 2022
CI Reliability 2022-06-20 nodejs/reliability#306
Open
CI Reliability 2022-06-21 nodejs/reliability#307
Open
@JakobJingleheimer JakobJingleheimer mentioned this pull request Jun 21, 2022
Revert "esm: convert resolve hook to synchronous" #43526
Merged
@GeoffreyBooth GeoffreyBooth added dont-land-on-v18.x and removed author ready PRs with CI started, the required approvals, and no outstanding review comments. labels Jun 22, 2022
@richardlau richardlau added dont-land-on-v14.x labels Jun 22, 2022
@JakobJingleheimer JakobJingleheimer mentioned this pull request Jun 23, 2022
esm: restore next<HookName>'s context as optional arg #43553
Merged
targos pushed a commit that referenced this pull request Jul 12, 2022
@aduh95 @targos
test: improve coverage for load hooks
48ae00c 
Refs: #43363 (comment)

PR-URL: #43374
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Akhil Marsonya <[email protected]>
targos pushed a commit that referenced this pull request Jul 19, 2022
@aduh95 @targos
test: improve coverage for load hooks
24dfd70 
Refs: #43363 (comment)

PR-URL: #43374
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Akhil Marsonya <[email protected]>
targos pushed a commit that referenced this pull request Jul 31, 2022
@aduh95 @targos
test: improve coverage for load hooks
222a6e9 
Refs: #43363 (comment)

PR-URL: #43374
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Akhil Marsonya <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Oct 4, 2022
2022-10-18, Version 19.0.0 (Current) #44626
Merged
RafaelGSS added a commit that referenced this pull request Oct 4, 2022
@RafaelGSS @ruyadorno
2022-10-18, Version 19.0.0 (Current)
ab691a5 
Notable Changes:

cli:
  * (SEMVER-MINOR) add `--watch` (Moshe Atlow) #44366
esm:
  * remove specifier resolution flag (Geoffrey Booth) #44859
  * convert `resolve` hook to synchronous (Jacob Smith) #43363
http:
  * (SEMVER-MINOR) add writeEarlyHints function to ServerResponse (Wing) #44180
  * (SEMVER-MAJOR) use Keep-Alive by default in global agents (Paolo Insogna) #43522
build:
  * (SEMVER-MAJOR) remove dtrace & etw support (Ben Noordhuis) #43652
  * (SEMVER-MAJOR) remove systemtap support (Ben Noordhuis) #43651

Deprecation and Removals:

* (SEMVER-MAJOR) runtime deprecate exports double slash maps (Guy Bedford) #44495

Semver-Major Commits:

* [aa3a572] - (SEMVER-MAJOR) build: remove dtrace & etw support (Ben Noordhuis) #43652
* [38f1e27] - (SEMVER-MAJOR) build: remove systemtap support (Ben Noordhuis) #43651
* [2849283] - (SEMVER-MAJOR) crypto: remove non-standard `webcrypto.Crypto.prototype.CryptoKey` (Antoine du Hamel) #42083
* [a1653ac] - (SEMVER-MAJOR) crypto: do not allow to call setFips from the worker thread (Sergey Petushkov) #43624
* [a4fa526] - (SEMVER-MAJOR) fs: add directory autodetection to fsPromises.symlink() (Livia Medeiros) #42894
* [bb4891d] - (SEMVER-MAJOR) fs: add validateBuffer to improve error (Hirotaka Tagawa / wafuwafu13) #44769
* [950a441] - (SEMVER-MAJOR) fs: remove coercion to string in writing methods (Livia Medeiros) #42796
* [41a6d82] - (SEMVER-MAJOR) fs: harden fs.readSync(buffer, options) typecheck (LiviaMedeiros) #42772
* [2275faa] - (SEMVER-MAJOR) fs: harden fs.read(params, callback) typecheck (LiviaMedeiros) #42772
* [29953a0] - (SEMVER-MAJOR) fs: harden filehandle.read(params) typecheck (LiviaMedeiros) #42772
* [4267b92] - (SEMVER-MAJOR) http: use Keep-Alive by default in global agents (Paolo Insogna) #43522
* [f529f73] - (SEMVER-MAJOR) lib: brand check event handler property receivers (Chengzhong Wu) #44483
* [6de2673] - (SEMVER-MAJOR) lib: enable global WebCrypto by default (Antoine du Hamel) #42083
* [73ba883] - (SEMVER-MAJOR) lib: use private field in AbortController (Joyee Cheung) #43820
* [7dd2f41] - (SEMVER-MAJOR) module: runtime deprecate exports double slash maps (Guy Bedford) #44495
* [587367d] - (SEMVER-MAJOR) perf_hooks: expose webperf global scope interfaces (Chengzhong Wu) #44483
* [364c0e1] - (SEMVER-MAJOR) perf_hooks: fix webperf idlharness (Chengzhong Wu) #44483
* [e0ab8dd] - (SEMVER-MAJOR) process: make process.config read only (Sergey Petushkov) #43627
* [481a959] - (SEMVER-MAJOR) readline: remove `question` method from `InterfaceConstructor` (Antoine du Hamel) #44606
* [77e5856] - (SEMVER-MAJOR) src: turn embedder api overload into default argument (Alena Khineika) #43629
* [dabda03] - (SEMVER-MAJOR) src: per-environment time origin value (Chengzhong Wu) #43781
* [2b32985] - (SEMVER-MAJOR) stream: use null for the error argument (Luigi Pinca) #44312
* [57ff476] - (SEMVER-MAJOR) test: remove duplicate test (Luigi Pinca) #44051
* [77def91] - (SEMVER-MAJOR) tls,http2: send fatal alert on ALPN mismatch (Tobias Nießen) #44031

PR-URL: #44626
Co-authored-by: Ruy Adorno <[email protected]>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
@aduh95 @guangwong
test: improve coverage for load hooks
02f9e05 
Refs: nodejs/node#43363 (comment)

PR-URL: nodejs/node#43374
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Akhil Marsonya <[email protected]>
RafaelGSS added a commit that referenced this pull request Oct 11, 2022
@RafaelGSS @ruyadorno
2022-10-18, Version 19.0.0 (Current)
f85f907 
Notable Changes:

cli:
  * (SEMVER-MINOR) add `--watch` (Moshe Atlow) #44366
esm:
  * remove specifier resolution flag (Geoffrey Booth) #44859
  * convert `resolve` hook to synchronous (Jacob Smith) #43363
http:
  * (SEMVER-MINOR) add writeEarlyHints function to ServerResponse (Wing) #44180
  * (SEMVER-MAJOR) use Keep-Alive by default in global agents (Paolo Insogna) #43522
build:
  * (SEMVER-MAJOR) remove dtrace & etw support (Ben Noordhuis) #43652
  * (SEMVER-MAJOR) remove systemtap support (Ben Noordhuis) #43651

Deprecation and Removals:

* (SEMVER-MAJOR) runtime deprecate exports double slash maps (Guy Bedford) #44495

Semver-Major Commits:

* [aa3a572] - (SEMVER-MAJOR) build: remove dtrace & etw support (Ben Noordhuis) #43652
* [38f1e27] - (SEMVER-MAJOR) build: remove systemtap support (Ben Noordhuis) #43651
* [2849283] - (SEMVER-MAJOR) crypto: remove non-standard `webcrypto.Crypto.prototype.CryptoKey` (Antoine du Hamel) #42083
* [a1653ac] - (SEMVER-MAJOR) crypto: do not allow to call setFips from the worker thread (Sergey Petushkov) #43624
* [a4fa526] - (SEMVER-MAJOR) fs: add directory autodetection to fsPromises.symlink() (Livia Medeiros) #42894
* [bb4891d] - (SEMVER-MAJOR) fs: add validateBuffer to improve error (Hirotaka Tagawa / wafuwafu13) #44769
* [950a441] - (SEMVER-MAJOR) fs: remove coercion to string in writing methods (Livia Medeiros) #42796
* [41a6d82] - (SEMVER-MAJOR) fs: harden fs.readSync(buffer, options) typecheck (LiviaMedeiros) #42772
* [2275faa] - (SEMVER-MAJOR) fs: harden fs.read(params, callback) typecheck (LiviaMedeiros) #42772
* [29953a0] - (SEMVER-MAJOR) fs: harden filehandle.read(params) typecheck (LiviaMedeiros) #42772
* [4267b92] - (SEMVER-MAJOR) http: use Keep-Alive by default in global agents (Paolo Insogna) #43522
* [f529f73] - (SEMVER-MAJOR) lib: brand check event handler property receivers (Chengzhong Wu) #44483
* [6de2673] - (SEMVER-MAJOR) lib: enable global WebCrypto by default (Antoine du Hamel) #42083
* [73ba883] - (SEMVER-MAJOR) lib: use private field in AbortController (Joyee Cheung) #43820
* [7dd2f41] - (SEMVER-MAJOR) module: runtime deprecate exports double slash maps (Guy Bedford) #44495
* [587367d] - (SEMVER-MAJOR) perf_hooks: expose webperf global scope interfaces (Chengzhong Wu) #44483
* [364c0e1] - (SEMVER-MAJOR) perf_hooks: fix webperf idlharness (Chengzhong Wu) #44483
* [e0ab8dd] - (SEMVER-MAJOR) process: make process.config read only (Sergey Petushkov) #43627
* [481a959] - (SEMVER-MAJOR) readline: remove `question` method from `InterfaceConstructor` (Antoine du Hamel) #44606
* [77e5856] - (SEMVER-MAJOR) src: turn embedder api overload into default argument (Alena Khineika) #43629
* [dabda03] - (SEMVER-MAJOR) src: per-environment time origin value (Chengzhong Wu) #43781
* [2b32985] - (SEMVER-MAJOR) stream: use null for the error argument (Luigi Pinca) #44312
* [57ff476] - (SEMVER-MAJOR) test: remove duplicate test (Luigi Pinca) #44051
* [77def91] - (SEMVER-MAJOR) tls,http2: send fatal alert on ALPN mismatch (Tobias Nießen) #44031

PR-URL: #44626
Co-authored-by: Ruy Adorno <[email protected]>
@ghost ghost mentioned this pull request Jul 10, 2023
javascript.operators.import_meta - import.meta.resolve is synchronous in Node.js v20 mdn/browser-compat-data#20324
Closed
This was referenced Jan 25, 2024
fix(vite-node): provide import.meta.filename and dirname vitest-dev/vitest#5011
Merged
fix(vitest): support older NodeJS with async import.meta.resolve vitest-dev/vitest#5045
Merged
@coderabbitai coderabbitai Bot mentioned this pull request Jul 28, 2026
test: stop rendered server process trees deterministically frankxai/frankx.ai-vercel-website#392
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ljharb ljharb ljharb left review comments
@jasnell jasnell jasnell left review comments
@guybedford guybedford guybedford left review comments
@GeoffreyBooth GeoffreyBooth GeoffreyBooth 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

esm Issues and PRs related to the ECMAScript Modules implementation. loaders Issues and PRs related to ES module loaders. notable-change PRs with changes that should be highlighted in changelogs.

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.

8 participants

@JakobJingleheimer @nodejs-github-bot @jasnell @GeoffreyBooth @ljharb @guybedford @aduh95 @richardlau
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.