FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

esm: add `--import` flag by MoLow · Pull Request #43942 · nodejs/node · GitHub

/ node Public

esm: add --import flag - #43942

Merged
nodejs-github-bot merged 3 commits into
nodejs:mainfrom
MoLow:add-experimental-import
Jul 31, 2022
ConversationCommits (3)ChecksFiles changed
Merged

esm: add --import flag#43942
nodejs-github-bot merged 3 commits into
nodejs:mainfrom
MoLow:add-experimental-import

Conversation

MoLow commented Jul 22, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Fixes: #40110

todo:

  • add tests

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

nodejs-github-bot commented Jul 22, 2022

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/modules

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
Comment thread doc/api/cli.md Outdated
Comment thread doc/api/cli.md Outdated
Comment thread lib/internal/process/esm_loader.js Outdated

This comment was marked as outdated.

Sign in to view

aduh95 commented Jul 22, 2022

Copy link
Copy Markdown
Contributor

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 commented Jul 22, 2022

Copy link
Copy Markdown
Contributor

Is this still spec compliant?

targos commented Jul 22, 2022

Copy link
Copy Markdown
Member

@MylesBorins What part of the spec is relevant?

GeoffreyBooth reviewed Jul 22, 2022
View reviewed changes

GeoffreyBooth left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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?

Comment thread src/node_options.cc Outdated
Comment thread src/node_options.cc Outdated

guybedford commented Jul 22, 2022

Copy link
Copy Markdown
Contributor

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.

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
Comment thread lib/internal/process/esm_loader.js Outdated
Comment thread lib/internal/process/esm_loader.js Outdated
Comment thread lib/internal/process/esm_loader.js Outdated
Comment thread lib/internal/process/esm_loader.js Outdated
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
Comment thread doc/api/cli.md Outdated

MoLow commented Jul 25, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

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

aduh95 reviewed Jul 25, 2022
View reviewed changes
Comment thread doc/api/cli.md Outdated
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
Comment thread doc/api/cli.md Outdated
Comment thread doc/api/cli.md Outdated
Comment thread doc/api/cli.md Outdated
Comment thread doc/api/cli.md Outdated
Comment thread test/es-module/test-esm-import-flag.js Outdated
Comment thread test/es-module/test-esm-import-flag.js Outdated
Comment thread test/fixtures/es-modules/print_imported.mjs Outdated
Comment thread lib/internal/process/esm_loader.js Outdated
aduh95 reviewed Jul 25, 2022
View reviewed changes
Comment thread test/es-module/test-esm-import-flag.js Outdated
Comment thread test/es-module/test-esm-import-flag.js Outdated
Comment thread test/es-module/test-esm-import-flag.js Outdated
Comment thread test/fixtures/es-modules/esm-top-level-await.mjs Outdated
Comment thread test/fixtures/es-modules/esm-top-level-await.mjs Outdated
Comment thread test/fixtures/es-modules/print_imported.mjs Outdated
Comment thread test/fixtures/es-modules/esm-top-level-await.mjs Outdated

MylesBorins commented Jul 25, 2022

Copy link
Copy Markdown
Contributor

@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 commented Jul 25, 2022

Copy link
Copy Markdown
Member

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.

aduh95 reviewed Jul 25, 2022
View reviewed changes
Comment thread test/es-module/test-esm-import-flag.js Outdated
Comment thread test/es-module/test-esm-loader-http-imports.mjs Outdated
Comment thread test/es-module/test-esm-loader-http-imports.mjs Outdated
Comment thread test/fixtures/es-modules/print_3.mjs Outdated
Comment thread test/fixtures/es-modules/esm-top-level-await.mjs Outdated
Comment thread test/fixtures/es-modules/esm-top-level-await.mjs Outdated
GeoffreyBooth reviewed Jul 26, 2022
View reviewed changes
Comment thread test/es-module/test-esm-import-flag.js Outdated
Comment thread test/fixtures/es-modules/print_3.mjs Outdated
Comment thread test/es-module/test-esm-loader-http-imports.mjs Outdated
Comment thread lib/internal/process/execution.js Outdated
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
This was referenced Nov 29, 2022
Closed
Closed
Closed
frank-dspeed mentioned this pull request Dec 4, 2022
Closed

ruyadorno commented Sep 7, 2023

Copy link
Copy Markdown
Member

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

GeoffreyBooth commented Sep 7, 2023

Copy link
Copy Markdown
Member

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

aduh95 pushed a commit to aduh95/node that referenced this pull request Sep 7, 2023
esm: add --import flag
31c6fcf 
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>
aduh95 mentioned this pull request Sep 7, 2023
Closed
ruyadorno pushed a commit that referenced this pull request Sep 8, 2023
esm: add --import flag
1f396d2 
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>
ruyadorno added a commit that referenced this pull request Sep 8, 2023
2023-09-14, Version 18.18.0 'Hydrogen' (LTS)
9f6929d 
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
ruyadorno mentioned this pull request Sep 8, 2023
Merged
ruyadorno added a commit that referenced this pull request Sep 10, 2023
2023-09-14, Version 18.18.0 'Hydrogen' (LTS)
6ddb80c 
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
ruyadorno added a commit that referenced this pull request Sep 11, 2023
2023-09-14, Version 18.18.0 'Hydrogen' (LTS)
1186fdb 
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
ruyadorno added a commit that referenced this pull request Sep 11, 2023
2023-09-14, Version 18.18.0 'Hydrogen' (LTS)
7c96325 
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
ruyadorno added a commit that referenced this pull request Sep 12, 2023
2023-09-14, Version 18.18.0 'Hydrogen' (LTS)
89e7fd1 
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
ruyadorno pushed a commit that referenced this pull request Sep 13, 2023
esm: add --import flag
c39b7c2 
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>
ruyadorno added a commit that referenced this pull request Sep 13, 2023
2023-09-14, Version 18.18.0 'Hydrogen' (LTS)
e873eef 
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
ruyadorno added a commit that referenced this pull request Sep 17, 2023
2023-09-18, Version 18.18.0 'Hydrogen' (LTS)
ab5fa2a 
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
ruyadorno added a commit that referenced this pull request Sep 18, 2023
2023-09-18, Version 18.18.0 'Hydrogen' (LTS)
78ab0ee 
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 reviewed Sep 19, 2023
View reviewed changes
Comment thread doc/api/cli.md
[#42511]: https://github.com/nodejs/node/issues/42511
[Chrome DevTools Protocol]: https://chromedevtools.github.io/devtools-protocol/
[CommonJS]: modules.md
[CommonJS module]: modules.md

styfle Sep 19, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks like you have two links to the same page here

daquinoaldo mentioned this pull request Feb 28, 2025
Closed
coderabbitai Bot mentioned this pull request Apr 13, 2026
Merged
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

ljharb ljharb left review comments
GeoffreyBooth GeoffreyBooth approved these changes
benjamingr benjamingr approved these changes
JakobJingleheimer JakobJingleheimer approved these changes
aduh95 aduh95 approved these changes
+2 more reviewers
styfle styfle left review comments
targos targos left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. esm Issues and PRs related to the ECMAScript Modules implementation. loaders Issues and PRs related to ES module loaders module Issues and PRs related to the module subsystem. process Issues and PRs related to the process subsystem. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add --import <module> flag for pre-loading ESM modules
Back | FazBrowse Home | New Git URL