esm: add `--import` flag #43942

MoLow · 2022-07-22T07:16:24Z

Fixes: #40110

todo:

add tests

nodejs-github-bot · 2022-07-22T07:16:28Z

Review requested:

@nodejs/modules

doc/api/cli.md

lib/internal/process/esm_loader.js

aduh95 · 2022-07-22T14:28:00Z

I thought this functionality is already possible with the globalPreload hook.

node/doc/api/esm.md

Lines 940 to 941 in d2fe72a

    
           scope that the application runs in. This hook allows the return of a string 
        
           that is run as a sloppy-mode script on startup.

--import is about loading ESM, so it's definitely not the same thing as sloppy-mode script. See #40110 if you need context on this feature.

MylesBorins · 2022-07-22T15:18:28Z

Is this still spec compliant?

targos · 2022-07-22T15:57:28Z

@MylesBorins What part of the spec is relevant?

GeoffreyBooth

This needs tests, including around variations of async behaviors:

An --import module that contains top-level await.
An --import module that does something async. Do we automatically await it before the next --import module runs? Before user code runs?

src/node_options.cc

guybedford · 2022-07-22T21:20:04Z

Overall I'm very much in favour of this, thanks for the PR!

I didn't follow the full thread, but what is the reasoning for starting with --experimental-import instead of just --import here? We can still label the feature as experimental without needing the experimental prefix IMO.

lib/internal/process/esm_loader.js

doc/api/cli.md

MoLow · 2022-07-25T14:28:57Z

added some tests, I am not sure if these are enough tests. please direct me to additional cases that need testing if I missed some.
in addition, I am ok with removing the --import from the flag name, just added it since it was what was written in the original issue
CC @GeoffreyBooth

doc/api/cli.md

test/es-module/test-esm-import-flag.js

test/fixtures/es-modules/print_imported.mjs

lib/internal/process/esm_loader.js

test/es-module/test-esm-import-flag.js

test/fixtures/es-modules/esm-top-level-await.mjs

test/fixtures/es-modules/print_imported.mjs

test/fixtures/es-modules/esm-top-level-await.mjs

MylesBorins · 2022-07-25T20:19:00Z

@MylesBorins What part of the spec is relevant?

I was curious about how this work in populating the internal cache and global state. Will this work similarly to subsequent script tags for example.

There might not be a spec issue, just wanted to confirm

GeoffreyBooth · 2022-07-25T21:11:24Z

Once this lands, I think we should mark it as blocked from release until we also land the “move loaders off thread” work, since that effort might effect --import. It would be similar to how we added the “backport-blocked-v18.x” tag to the chaining loaders PR until we had a few loaders-related PRs ready to release together. Thoughts? I know this will be experimental but I’m hopeful that the other PR isn’t too far behind and therefore we could release these together and minimize churn.

test/es-module/test-esm-import-flag.js

test/es-module/test-esm-loader-http-imports.mjs

test/fixtures/es-modules/print_3.mjs

test/fixtures/es-modules/esm-top-level-await.mjs

test/es-module/test-esm-import-flag.js

test/fixtures/es-modules/print_3.mjs

test/es-module/test-esm-loader-http-imports.mjs

lib/internal/process/execution.js

ruyadorno · 2023-09-07T00:36:43Z

This will need manual backport in case we want to land it in v18.

GeoffreyBooth · 2023-09-07T00:59:01Z

Yes, we need this on 18 to support the module customization hooks.

PR-URL: nodejs#43942 Fixes: nodejs#40110 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

PR-URL: #43942 Backport-PR-URL: #49539 Fixes: #40110 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

Notable changes: crypto: * update root certificates to NSS 3.90 (Node.js GitHub Bot) #48416 doc: * add kvakil to collaborators (Keyhan Vakil) #48449 esm: * (SEMVER-MINOR) add `--import` flag (Moshe Atlow) #43942 net: * add autoSelectFamily global getter and setter (Paolo Insogna) #45777 url: * (SEMVER-MINOR) add value argument to has and delete methods (Sankalp Shubham) #47885 PR-URL: #49220

Notable changes: crypto: * update root certificates to NSS 3.90 (Node.js GitHub Bot) #48416 doc: * add vmoroz to collaborators (Vladimir Morozov) #48527 * add kvakil to collaborators (Keyhan Vakil) #48449 esm: * (SEMVER-MINOR) add `--import` flag (Moshe Atlow) #43942 net: * add autoSelectFamily global getter and setter (Paolo Insogna) #45777 url: * (SEMVER-MINOR) add value argument to has and delete methods (Sankalp Shubham) #47885 PR-URL: #49220

Notable changes: crypto: * update root certificates to NSS 3.90 (Node.js GitHub Bot) #48416 doc: * add vmoroz to collaborators (Vladimir Morozov) #48527 * add kvakil to collaborators (Keyhan Vakil) #48449 esm: * (SEMVER-MINOR) add `--import` flag (Moshe Atlow) #43942 events: * (SEMVER-MINOR) allow safely adding listener to abortSignal (Chemi Atlow) #48596 net: * add autoSelectFamily global getter and setter (Paolo Insogna) #45777 url: * (SEMVER-MINOR) add value argument to has and delete methods (Sankalp Shubham) #47885 PR-URL: #49220

Notable changes: build: * sync libuv header change (Jiawen Geng) #48078 crypto: * update root certificates to NSS 3.90 (Node.js GitHub Bot) #48416 deps: * add missing thread-common.c in uv.gyp (Santiago Gimeno) #48078 * upgrade to libuv 1.46.0 (Santiago Gimeno) #48078 * upgrade to libuv 1.45.0 (Santiago Gimeno) #48078 doc: * add vmoroz to collaborators (Vladimir Morozov) #48527 * add kvakil to collaborators (Keyhan Vakil) #48449 esm: * (SEMVER-MINOR) add `--import` flag (Moshe Atlow) #43942 events: * (SEMVER-MINOR) allow safely adding listener to abortSignal (Chemi Atlow) #48596 fs, stream: * initial `Symbol.dispose` and `Symbol.asyncDispose` support (Moshe Atlow) #48518 net: * add autoSelectFamily global getter and setter (Paolo Insogna) #45777 url: * (SEMVER-MINOR) add value argument to has and delete methods (Sankalp Shubham) #47885 PR-URL: #49220

Notable changes: build: * sync libuv header change (Jiawen Geng) #48078 crypto: * update root certificates to NSS 3.93 (Node.js GitHub Bot) #49341 * update root certificates to NSS 3.90 (Node.js GitHub Bot) #48416 deps: * add missing thread-common.c in uv.gyp (Santiago Gimeno) #48078 * upgrade to libuv 1.46.0 (Santiago Gimeno) #48078 * upgrade to libuv 1.45.0 (Santiago Gimeno) #48078 doc: * add atlowChemi to collaborators (atlowChemi) #48757 * add vmoroz to collaborators (Vladimir Morozov) #48527 * add kvakil to collaborators (Keyhan Vakil) #48449 esm: * (SEMVER-MINOR) add `--import` flag (Moshe Atlow) #43942 events: * (SEMVER-MINOR) allow safely adding listener to abortSignal (Chemi Atlow) #48596 fs, stream: * initial `Symbol.dispose` and `Symbol.asyncDispose` support (Moshe Atlow) #48518 net: * add autoSelectFamily global getter and setter (Paolo Insogna) #45777 url: * (SEMVER-MINOR) add value argument to has and delete methods (Sankalp Shubham) #47885 PR-URL: #49220

PR-URL: #43942 Backport-PR-URL: #49539 Fixes: #40110 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

Notable changes: build: * sync libuv header change (Jiawen Geng) #48078 crypto: * update root certificates to NSS 3.93 (Node.js GitHub Bot) #49341 * update root certificates to NSS 3.90 (Node.js GitHub Bot) #48416 deps: * add missing thread-common.c in uv.gyp (Santiago Gimeno) #48078 * upgrade to libuv 1.46.0 (Santiago Gimeno) #48078 * upgrade to libuv 1.45.0 (Santiago Gimeno) #48078 doc: * add atlowChemi to collaborators (atlowChemi) #48757 * add vmoroz to collaborators (Vladimir Morozov) #48527 * add kvakil to collaborators (Keyhan Vakil) #48449 esm: * (SEMVER-MINOR) add `--import` flag (Moshe Atlow) #43942 events: * (SEMVER-MINOR) allow safely adding listener to abortSignal (Chemi Atlow) #48596 fs, stream: * initial `Symbol.dispose` and `Symbol.asyncDispose` support (Moshe Atlow) #48518 net: * add autoSelectFamily global getter and setter (Paolo Insogna) #45777 url: * (SEMVER-MINOR) add value argument to has and delete methods (Sankalp Shubham) #47885 PR-URL: #49220

styfle · 2023-09-19T19:39:35Z

doc/api/cli.md

@@ -2086,7 +2101,9 @@ done
 [#42511]: https://github.com/nodejs/node/issues/42511
 [Chrome DevTools Protocol]: https://chromedevtools.github.io/devtools-protocol/
 [CommonJS]: modules.md
+[CommonJS module]: modules.md


Looks like you have two links to the same page here

Notable changes: build: * sync libuv header change (Jiawen Geng) nodejs#48078 crypto: * update root certificates to NSS 3.93 (Node.js GitHub Bot) nodejs#49341 * update root certificates to NSS 3.90 (Node.js GitHub Bot) nodejs#48416 deps: * add missing thread-common.c in uv.gyp (Santiago Gimeno) nodejs#48078 * upgrade to libuv 1.46.0 (Santiago Gimeno) nodejs#48078 * upgrade to libuv 1.45.0 (Santiago Gimeno) nodejs#48078 doc: * add atlowChemi to collaborators (atlowChemi) nodejs#48757 * add vmoroz to collaborators (Vladimir Morozov) nodejs#48527 * add kvakil to collaborators (Keyhan Vakil) nodejs#48449 esm: * (SEMVER-MINOR) add `--import` flag (Moshe Atlow) nodejs#43942 events: * (SEMVER-MINOR) allow safely adding listener to abortSignal (Chemi Atlow) nodejs#48596 fs, stream: * initial `Symbol.dispose` and `Symbol.asyncDispose` support (Moshe Atlow) nodejs#48518 net: * add autoSelectFamily global getter and setter (Paolo Insogna) nodejs#45777 url: * (SEMVER-MINOR) add value argument to has and delete methods (Sankalp Shubham) nodejs#47885 PR-URL: nodejs#49220

MoLow force-pushed the add-experimental-import branch from 06de59d to 198f88f Compare July 22, 2022 07:16

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem. labels Jul 22, 2022

MoLow added the esm Issues and PRs related to the ECMAScript Modules implementation. label Jul 22, 2022

aduh95 reviewed Jul 22, 2022

View reviewed changes

doc/api/cli.md Outdated Show resolved Hide resolved

doc/api/cli.md Outdated Show resolved Hide resolved

lib/internal/process/esm_loader.js Outdated Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

GeoffreyBooth reviewed Jul 22, 2022

View reviewed changes

src/node_options.cc Outdated Show resolved Hide resolved

src/node_options.cc Outdated Show resolved Hide resolved

MoLow force-pushed the add-experimental-import branch from 198f88f to 5a15e7a Compare July 23, 2022 21:50

JakobJingleheimer reviewed Jul 24, 2022

View reviewed changes

lib/internal/process/esm_loader.js Outdated Show resolved Hide resolved

lib/internal/process/esm_loader.js Outdated Show resolved Hide resolved

lib/internal/process/esm_loader.js Outdated Show resolved Hide resolved

lib/internal/process/esm_loader.js Outdated Show resolved Hide resolved

MoLow force-pushed the add-experimental-import branch from dfad8f5 to a7d6bae Compare July 25, 2022 14:22

aduh95 reviewed Jul 25, 2022

View reviewed changes

doc/api/cli.md Outdated Show resolved Hide resolved

aduh95 reviewed Jul 25, 2022

View reviewed changes

doc/api/cli.md Outdated Show resolved Hide resolved

MoLow changed the title ~~esm: add --experimental-import flag~~ esm: add --import flag Jul 25, 2022

MoLow added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Jul 25, 2022

GeoffreyBooth reviewed Jul 25, 2022

View reviewed changes

aduh95 reviewed Jul 25, 2022

View reviewed changes

GeoffreyBooth reviewed Jul 26, 2022

View reviewed changes

GeoffreyBooth added module Issues and PRs related to the module subsystem. loaders Issues and PRs related to ES module loaders labels Jul 26, 2022

MoLow requested a review from benjamingr July 26, 2022 16:04

lachrist mentioned this pull request Sep 23, 2022

Loader getappmap/appmap-agent-js#123

Closed

MoLow mentioned this pull request Sep 29, 2022

[v18.x backport] cli: add --watch #44815

Closed

This was referenced Nov 29, 2022

Proposal: modify cli to nudge People to do the right thing in nodejs 19.1+ microsoft/TypeScript#51681

Closed

Verify NJS 19,1 import require flags lemanschik/modules#2

Closed

PR: cli api just-js/just#56

Closed

frank-dspeed mentioned this pull request Dec 4, 2022

Add Support for Top Level Await inside CJS oracle/graaljs#350

Closed

aduh95 removed the backport-blocked-v18.x PRs that should land on the v18.x-staging branch but are blocked by another PR's pending backport. label Sep 6, 2023

ruyadorno added the backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. label Sep 7, 2023

aduh95 mentioned this pull request Sep 7, 2023

[v18.x backport] esm: add --import flag #49539

Closed

aduh95 added backport-open-v18.x Indicate that the PR has an open backport. and removed backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. labels Sep 8, 2023

ruyadorno mentioned this pull request Sep 8, 2023

v18.18.0 proposal #49220

Merged

ruyadorno added backported-to-v18.x PRs backported to the v18.x-staging branch. and removed backport-open-v18.x Indicate that the PR has an open backport. labels Sep 9, 2023

styfle reviewed Sep 19, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

esm: add `--import` flag #43942

esm: add `--import` flag #43942

MoLow commented Jul 22, 2022 •

edited

nodejs-github-bot commented Jul 22, 2022

This comment was marked as outdated.

aduh95 commented Jul 22, 2022

MylesBorins commented Jul 22, 2022

targos commented Jul 22, 2022

GeoffreyBooth left a comment

guybedford commented Jul 22, 2022

MoLow commented Jul 25, 2022 •

edited

MylesBorins commented Jul 25, 2022

GeoffreyBooth commented Jul 25, 2022

ruyadorno commented Sep 7, 2023

GeoffreyBooth commented Sep 7, 2023

styfle Sep 19, 2023

esm: add --import flag #43942

esm: add --import flag #43942

Conversation

MoLow commented Jul 22, 2022 • edited

nodejs-github-bot commented Jul 22, 2022

This comment was marked as outdated.

aduh95 commented Jul 22, 2022

MylesBorins commented Jul 22, 2022

targos commented Jul 22, 2022

GeoffreyBooth left a comment

Choose a reason for hiding this comment

guybedford commented Jul 22, 2022

MoLow commented Jul 25, 2022 • edited

MylesBorins commented Jul 25, 2022

GeoffreyBooth commented Jul 25, 2022

ruyadorno commented Sep 7, 2023

GeoffreyBooth commented Sep 7, 2023

styfle Sep 19, 2023

Choose a reason for hiding this comment

esm: add `--import` flag #43942

esm: add `--import` flag #43942

MoLow commented Jul 22, 2022 •

edited

MoLow commented Jul 25, 2022 •

edited