esm: convert `resolve` hook to synchronous#43363

JakobJingleheimer · 2022-06-09T18:15:44Z

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

nodejs-github-bot · 2022-06-09T18:15:48Z

Review requested:

@nodejs/modules

lib/internal/modules/esm/resolve.js

lib/internal/modules/esm/loader.js

test/es-module/test-esm-import-meta-resolve.mjs

test/es-module/test-esm-loader-search.js

Refs: nodejs#43363 (comment)

guybedford

Would love to see this!

lib/internal/modules/esm/initialize_import_meta.js

lib/internal/modules/esm/loader.js

test/es-module/test-esm-import-meta-resolve.mjs

jasnell · 2022-06-14T14:22:10Z

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.

nodejs-github-bot · 2022-06-18T17:22:59Z

Landed in 90b634a

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]>

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]>

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]>

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]>

esm: convert resolve hook to synchronous
37abe52

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 added the needs-ci PRs that need a full CI run. label Jun 9, 2022

GeoffreyBooth reviewed Jun 9, 2022
View reviewed changes

lib/internal/modules/esm/resolve.jsShow resolvedHide resolved

aduh95 reviewed Jun 10, 2022
View reviewed changes

lib/internal/modules/esm/loader.js Outdated Show resolvedHide resolved

aduh95 reviewed Jun 10, 2022
View reviewed changes

lib/internal/modules/esm/loader.js Outdated Show resolvedHide resolved

aduh95 reviewed Jun 10, 2022
View reviewed changes

lib/internal/modules/esm/loader.js Outdated Show resolvedHide resolved

aduh95 reviewed Jun 10, 2022
View reviewed changes

test/es-module/test-esm-import-meta-resolve.mjs Outdated Show resolvedHide resolved

aduh95 reviewed Jun 10, 2022
View reviewed changes

test/es-module/test-esm-loader-search.js Outdated Show resolvedHide resolved

aduh95 added a commit to aduh95/node that referenced this pull request Jun 11, 2022
test: improve coverage for load hooks
c7be4b7
Refs: nodejs#43363 (comment)

aduh95 added a commit to aduh95/node that referenced this pull request Jun 11, 2022
test: improve coverage for load hooks
d3789b5
Refs: nodejs#43363 (comment)

aduh95 mentioned this pull request Jun 11, 2022
test: improve coverage for load hooks #43374
Merged

guybedford reviewed Jun 11, 2022
View reviewed changes

lib/internal/modules/esm/initialize_import_meta.jsShow resolvedHide resolved

aduh95 reviewed Jun 12, 2022
View reviewed changes

lib/internal/modules/esm/loader.js Outdated Show resolvedHide resolved

aduh95 reviewed Jun 12, 2022
View reviewed changes

lib/internal/modules/esm/loader.js Outdated Show resolvedHide resolved

aduh95 reviewed Jun 12, 2022
View reviewed changes

test/es-module/test-esm-import-meta-resolve.mjs Outdated Show resolvedHide resolved

JakobJingleheimer added 3 commits June 12, 2022 16:01

fixup: fix import.meta.resolve
edb6ba5

fixup: correct assert.throws()
ae8ef63

fixup: support detecting when a resolve hook returns a promise
8bf7965

JakobJingleheimer force-pushed the esm/synchronous-resolve-hook branch 2 times, most recently from 8425665 to c3b979cCompare June 12, 2022 18:24

fixup: change async/thenable detection
ab935b6

JakobJingleheimer force-pushed the esm/synchronous-resolve-hook branch from c3b979c to ab935b6Compare June 12, 2022 18:38

JakobJingleheimer added 2 commits June 12, 2022 20:44

fixup: update ESM doc for resolve hook
28d813f

fixup: revert back to arg destructuring
b332b97

This comment was marked as resolved.
Sign in to view

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

JakobJingleheimer added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 18, 2022

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 18, 2022

nodejs-github-bot merged commit 90b634a into nodejs:mainJun 18, 2022

GeoffreyBooth deleted the esm/synchronous-resolve-hook branch June 18, 2022 17:51

GeoffreyBooth removed needs-ci PRs that need a full CI run. commit-queue-failed An error occurred while landing this pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Jun 18, 2022

github-actionsbot 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 mentioned this pull request Jun 21, 2022
Revert "esm: convert resolve hook to synchronous" #43526
Merged

GeoffreyBooth added dont-land-on-v18.x and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Jun 22, 2022

richardlau added dont-land-on-v14.x labels Jun 22, 2022

JakobJingleheimer mentioned this pull request Jun 23, 2022
esm: restore next<HookName>'s context as optional arg #43553
Merged

RafaelGSS mentioned this pull request Oct 4, 2022
2022-10-18, Version 19.0.0 (Current) #44626
Merged

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.resolvevitest-dev/vitest#5045
Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

esm: convert `resolve` hook to synchronous#43363

esm: convert `resolve` hook to synchronous #43363

Uh oh!

JakobJingleheimer commented Jun 9, 2022•
edited
Loading

Uh oh!

nodejs-github-bot commented Jun 9, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

guybedford left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

jasnell commented Jun 14, 2022

Uh oh!

nodejs-github-bot commented Jun 18, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Uh oh!

esm: convert resolve hook to synchronous#43363

esm: convert resolve hook to synchronous #43363

Uh oh!

Conversation

JakobJingleheimer commented Jun 9, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Notable changes

Unblocks

Uh oh!

nodejs-github-bot commented Jun 9, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

guybedford left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

jasnell commented Jun 14, 2022

Uh oh!

nodejs-github-bot commented Jun 18, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

esm: convert `resolve` hook to synchronous#43363

esm: convert `resolve` hook to synchronous #43363

JakobJingleheimer commented Jun 9, 2022•
edited
Loading