esm: unflag import.meta.resolve #49028

guybedford · 2023-08-05T17:43:26Z

Unflags import.meta.resolve making the second parentURL argument still gated behind the same --experimental-import-meta-resolve flag so that existing users would retain backwards compatibility, while the default behaviour becomes standards compliant.

If we can combine this approach with supporting file:/// URLs in fetch, that would be a powerful new approach for cross-platform asset loading - await (await fetch(import.meta.resolve('./asset.bin'))).arrayBuffer() which hopefully in turn bundlers should also be able to support better too.

//cc @nodejs/modules

nodejs-github-bot · 2023-08-05T17:43:30Z

Review requested:

@nodejs/loaders
@nodejs/modules

GeoffreyBooth

Looks good, but should we add some docs to esm.md?

lib/internal/modules/esm/initialize_import_meta.js

ljharb · 2023-08-05T19:24:30Z

The last time I tried this it had a ton of bugs and gaps; can you confirm that anything that import() or import can reach, import.meta.resolve can produce a URL for?

guybedford · 2023-08-06T01:43:19Z

@ljharb right, as pointed out in #49010, this still seemed to be the case. I've pushed a fix with this PR and updated the description to note that we now always provide the resolution when exact modules are not found, while not found package names will still throw not found.

GeoffreyBooth · 2023-08-06T02:13:25Z

I feel like bugfixes should go in their own PR, not be coupled with unflagging.

I don’t think bugs need to necessarily hold up unflagging. We fix bugs in unflagged/stable APIs all the time. If there are any other known open bugs related to import.meta.resolve, it would be good to fix them before unflagging, but if not then I’m fine with unflagging and continuing to bugfix as users report issues.

ljharb · 2023-08-06T02:31:40Z

Will it remain experimental despite being unflagged?

guybedford · 2023-08-06T03:03:42Z

I can split the changes out if it would be preferred, but I think the only thing that would really affect is backporting - and I think it would be fine to backport this change without a concern too.

doc/api/esm.md

guybedford · 2023-08-06T03:08:53Z

I've split out the not found fix into #49038, so this should now land after that.

JakobJingleheimer

🙌 Good call converting the flag to affect only parentURL

lib/internal/modules/esm/initialize_import_meta.js

nodejs-github-bot · 2023-08-12T05:17:00Z

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

nodejs-github-bot · 2023-08-12T10:21:20Z

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

GeoffreyBooth · 2023-08-13T00:02:01Z

@guybedford I think what's happening in the ASan job that's failing is that the overall test file is timing out. Each subtest is taking 5 seconds or so. Perhaps this is a real error.

Co-authored-by: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com>

Notable changes: esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 PR-URL: #49185

PR-URL: nodejs#49028 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>

Notable changes: doc: * add new TSC members (Michael Dawson) #48841 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 PR-URL: #49185

PR-URL: #49028 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>

Notable changes: doc: * add new TSC members (Michael Dawson) #48841 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 PR-URL: #49185

Notable changes: doc: * add new TSC members (Michael Dawson) #48841 * add rluvaton to collaborators (Raz Luvaton) #49215 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 src: * add built-in `.env` file support (Yagiz Nizipli) #48890 PR-URL: #49185

Notable changes: deps: * V8: cherry-pick 93275031284c (Joyee Cheung) #48660 doc: * add new TSC members (Michael Dawson) #48841 * add rluvaton to collaborators (Raz Luvaton) #49215 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 src: * add built-in `.env` file support (Yagiz Nizipli) #48890 PR-URL: #49185

deps: * V8: cherry-pick 93275031284c (Joyee Cheung) #48660 doc: * add new TSC members (Michael Dawson) #48841 * add rluvaton to collaborators (Raz Luvaton) #49215 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 src: * add built-in `.env` file support (Yagiz Nizipli) #48890 * initialize cppgc (Daryl Haresign and Joyee Cheung) #48660 and #45704 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 PR-URL: #49185

Notable changes: deps: * V8: cherry-pick 93275031284c (Joyee Cheung) #48660 doc: * add new TSC members (Michael Dawson) #48841 * add rluvaton to collaborators (Raz Luvaton) #49215 esm: * unflag import.meta.resolve (Guy Bedford) #49028 * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842 * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559 inspector: * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765 module: * implement `register` utility (João Lenon) #46826 * make CJS load from ESM loader (Antoine du Hamel) #47999 src: * add built-in `.env` file support (Yagiz Nizipli) #48890 * initialize cppgc (Daryl Haresign and Joyee Cheung) #48660 and #45704 test_runner: * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 PR-URL: #49185

PR-URL: #49247 Refs: #49028 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Guy Bedford <guybedford@gmail.com>

import.meta.resolve was unflagged in Node.js 20.6 by nodejs/node#49028 Unfortunately, the unflagged version does not support the parentURL argument. Rather than attempting to detect whether the argument is supported (which is not as simple as .length testing) stop using it until a reliable version is unflagged. Signed-off-by: Kevin Locke <kevin@kevinlocke.name>

PR-URL: #49247 Refs: #49028 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Guy Bedford <guybedford@gmail.com>

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. labels Aug 5, 2023

GeoffreyBooth approved these changes Aug 5, 2023

View reviewed changes

anonrig reviewed Aug 5, 2023

View reviewed changes

lib/internal/modules/esm/initialize_import_meta.js Outdated Show resolved Hide resolved

GeoffreyBooth approved these changes Aug 5, 2023

View reviewed changes

guybedford mentioned this pull request Aug 6, 2023

import.meta.resolve non existing paths. #49010

Closed

guybedford commented Aug 6, 2023

View reviewed changes

doc/api/esm.md Show resolved Hide resolved

guybedford force-pushed the import-meta-resolve-unflag branch from 5e75ebd to 36a1f5b Compare August 6, 2023 03:09

GeoffreyBooth approved these changes Aug 6, 2023

View reviewed changes

JakobJingleheimer approved these changes Aug 6, 2023

View reviewed changes

lib/internal/modules/esm/initialize_import_meta.js Outdated Show resolved Hide resolved

VoltrexKeyva approved these changes Aug 9, 2023

View reviewed changes

GeoffreyBooth removed the needs-ci PRs that need a full CI run. label Aug 12, 2023

github-actions bot mentioned this pull request Aug 13, 2023

CI Reliability 2023-08-13 nodejs/reliability#635

Open

29 tasks

guybedford and others added 6 commits August 12, 2023 19:47

esm: unflag import.meta.resolve

7797444

update pr url

719c418

replaceme

1c5c2d8

docs changes

f865a1d

pr feedback

69796fd

Update lib/internal/modules/esm/initialize_import_meta.js

09f2bef

Co-authored-by: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com>

aduh95 mentioned this pull request Aug 19, 2023

doc: edit import.meta.resolve documentation #49247

Merged

nodejs-github-bot pushed a commit that referenced this pull request Sep 5, 2023

doc: edit import.meta.resolve documentation

e11c7b7

PR-URL: #49247 Refs: #49028 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Guy Bedford <guybedford@gmail.com>

koshic mentioned this pull request Sep 6, 2023

Breakage in Node.js v20.6.0 iambumblehead/esmock#234

Closed

nicolo-ribaudo mentioned this pull request Sep 8, 2023

Fix compatibility with Node.js 20.6 babel/babel#15947

Merged

ruyadorno pushed a commit that referenced this pull request Sep 28, 2023

doc: edit import.meta.resolve documentation

b9d4a80

PR-URL: #49247 Refs: #49028 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Guy Bedford <guybedford@gmail.com>

esm: unflag import.meta.resolve #49028

esm: unflag import.meta.resolve #49028

guybedford commented Aug 5, 2023 •

edited

nodejs-github-bot commented Aug 5, 2023

GeoffreyBooth left a comment

ljharb commented Aug 5, 2023

guybedford commented Aug 6, 2023

GeoffreyBooth commented Aug 6, 2023

ljharb commented Aug 6, 2023

guybedford commented Aug 6, 2023

guybedford commented Aug 6, 2023

JakobJingleheimer left a comment

nodejs-github-bot commented Aug 12, 2023

nodejs-github-bot commented Aug 12, 2023

GeoffreyBooth commented Aug 13, 2023

esm: unflag import.meta.resolve #49028

esm: unflag import.meta.resolve #49028

Conversation

guybedford commented Aug 5, 2023 • edited

nodejs-github-bot commented Aug 5, 2023

GeoffreyBooth left a comment

Choose a reason for hiding this comment

ljharb commented Aug 5, 2023

guybedford commented Aug 6, 2023

GeoffreyBooth commented Aug 6, 2023

ljharb commented Aug 6, 2023

guybedford commented Aug 6, 2023

guybedford commented Aug 6, 2023

JakobJingleheimer left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Aug 12, 2023

nodejs-github-bot commented Aug 12, 2023

GeoffreyBooth commented Aug 13, 2023

guybedford commented Aug 5, 2023 •

edited