esm: update loaders warning #49633

GeoffreyBooth · 2023-09-13T04:59:11Z

Following up #49597, this PR edits the warning when --experimental-loader / --loader is used to tell people to use --import / register() instead, which has no warning. The separate warning about globalPreload remains.

nodejs-github-bot · 2023-09-13T04:59:16Z

Review requested:

@nodejs/loaders

aduh95 · 2023-09-13T08:47:41Z

Can we keep the warning for use of --loader? This one is not RC IIUC, so it should still have a warning.

GeoffreyBooth · 2023-09-14T00:00:20Z

Can we keep the warning for use of --loader? This one is not RC IIUC, so it should still have a warning.

It turns out the previous warning was only for --loader / --experimental-loader, not for register. So people already using the new --import / register flow aren’t seeing any warnings. I updated this to change the warning to push people in that direction. @aduh95

./node --loader ./test/fixtures/es-module-loaders/never-settling-resolve-step/loader.mjs \
  ./test/fixtures/es-module-loaders/never-settling-resolve-step/race.cjs

(node:49878) ExperimentalWarning: `--experimental-loader` may be removed in the future;
  instead use `--import` to reference a file that calls `register()`

test/es-module/test-esm-experimental-warnings.mjs

lib/internal/modules/esm/loader.js

test/es-module/test-esm-experimental-warnings.mjs

nodejs-github-bot · 2023-09-16T15:51:14Z

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

lib/internal/modules/esm/loader.js

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

lib/internal/modules/esm/loader.js

nodejs-github-bot · 2023-09-18T13:40:44Z

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

nodejs-github-bot · 2023-09-18T16:05:36Z

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

nodejs-github-bot · 2023-09-18T17:08:58Z

Commit Queue failed

- Loading data for nodejs/node/pull/49633
✔  Done loading data for nodejs/node/pull/49633
----------------------------------- PR info ------------------------------------
Title      esm: update loaders warning (#49633)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     GeoffreyBooth:remove-loaders-warning -> nodejs:main
Labels     experimental, esm, author ready, needs-ci, loaders, commit-queue-squash
Commits    7
 - esm: update loaders warning
 - Change warning to be specific to --experimental-loader
 - Update test/es-module/test-esm-experimental-warnings.mjs
 - Update test/es-module/test-esm-experimental-warnings.mjs
 - Improve warning
 - lint
 - `readableURIEncode`
Committers 1
 - Antoine du Hamel 
PR-URL: https://github.com/nodejs/node/pull/49633
Reviewed-By: Jacob Smith 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: James M Snell 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/49633
Reviewed-By: Jacob Smith 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: James M Snell 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - esm: update loaders warning
   ⚠  - Change warning to be specific to --experimental-loader
   ⚠  - Update test/es-module/test-esm-experimental-warnings.mjs
   ⚠  - Update test/es-module/test-esm-experimental-warnings.mjs
   ⚠  - Improve warning
   ⚠  - lint
   ⚠  - `readableURIEncode`
   ℹ  This PR was created on Wed, 13 Sep 2023 04:59:11 GMT
   ✔  Approvals: 3
   ✔  - Jacob Smith (@JakobJingleheimer): https://github.com/nodejs/node/pull/49633#pullrequestreview-1623900278
   ✔  - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/49633#pullrequestreview-1625510070
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/49633#pullrequestreview-1629979006
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2023-09-18T16:05:36Z: https://ci.nodejs.org/job/node-test-pull-request/54041/
- Querying data for job/node-test-pull-request/54041/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/6225715618

guybedford

Nice to see the progress here. I'd love to see a short flag for --import at some point as well.

lib/internal/modules/esm/loader.js

nodejs-github-bot · 2023-09-19T03:59:51Z

Landed in f91b4e2

PR-URL: #49633 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Guy Bedford <guybedford@gmail.com>

PR-URL: nodejs#49633 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Guy Bedford <guybedford@gmail.com>

PR-URL: #49633 Backport-PR-URL: #50669 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Guy Bedford <guybedford@gmail.com>

PR-URL: nodejs/node#49633 Backport-PR-URL: nodejs/node#50669 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Guy Bedford <guybedford@gmail.com>

GeoffreyBooth added experimental Issues and PRs related to experimental features. esm Issues and PRs related to the ECMAScript Modules implementation. loaders Issues and PRs related to ES module loaders labels Sep 13, 2023

GeoffreyBooth requested review from JakobJingleheimer and aduh95 September 13, 2023 04:59

nodejs-github-bot added the needs-ci PRs that need a full CI run. label Sep 13, 2023

JakobJingleheimer approved these changes Sep 13, 2023

View reviewed changes

benjamingr approved these changes Sep 13, 2023

View reviewed changes

GeoffreyBooth changed the title ~~esm: remove loaders warning~~ esm: update loaders warning Sep 14, 2023

GeoffreyBooth force-pushed the remove-loaders-warning branch from 601890e to 70b1a64 Compare September 14, 2023 00:01

aduh95 reviewed Sep 14, 2023

View reviewed changes

test/es-module/test-esm-experimental-warnings.mjs Outdated Show resolved Hide resolved

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

aduh95 reviewed Sep 14, 2023

View reviewed changes

test/es-module/test-esm-experimental-warnings.mjs Outdated Show resolved Hide resolved

aduh95 mentioned this pull request Sep 14, 2023

esm: fix support for URL instances in register #49655

Merged

jasnell approved these changes Sep 16, 2023

View reviewed changes

aduh95 reviewed Sep 16, 2023

View reviewed changes

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

GeoffreyBooth and others added 7 commits September 16, 2023 23:10

esm: update loaders warning

f471450

Change warning to be specific to --experimental-loader

69371c9

Update test/es-module/test-esm-experimental-warnings.mjs

cf082a9

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

Update test/es-module/test-esm-experimental-warnings.mjs

42077e5

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

Improve warning

edc985e

lint

27c14d7

readableURIEncode

45d517d

aduh95 force-pushed the remove-loaders-warning branch from 24f23b5 to 45d517d Compare September 16, 2023 21:28

github-actions bot mentioned this pull request Sep 17, 2023

CI Reliability 2023-09-17 nodejs/reliability#668

Open

32 tasks

GeoffreyBooth commented Sep 17, 2023

View reviewed changes

lib/internal/modules/esm/loader.js Show resolved Hide resolved

github-actions bot mentioned this pull request Sep 18, 2023

CI Reliability 2023-09-18 nodejs/reliability#669

Open

33 tasks

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Sep 18, 2023

github-actions bot mentioned this pull request Sep 19, 2023

CI Reliability 2023-09-19 nodejs/reliability#670

Open

32 tasks

guybedford reviewed Sep 19, 2023

View reviewed changes

lib/internal/modules/esm/loader.js Show resolved Hide resolved

guybedford approved these changes Sep 19, 2023

View reviewed changes

GeoffreyBooth added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Sep 19, 2023

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 19, 2023

nodejs-github-bot merged commit f91b4e2 into nodejs:main Sep 19, 2023
36 checks passed

github-actions bot mentioned this pull request Sep 20, 2023

CI Reliability 2023-09-20 nodejs/reliability#671

Open

27 tasks

This was referenced Sep 28, 2023

v20.7.1 proposal #49917

Closed

v20.8.0 proposal #49932

Merged

targos mentioned this pull request Nov 11, 2023

[v18.x] Backport most ESM and customization hook changes #50669

Closed

targos added the backported-to-v18.x PRs backported to the v18.x-staging branch. label Nov 23, 2023

targos mentioned this pull request Nov 28, 2023

v18.19.0 release proposal #50953

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

esm: update loaders warning #49633

esm: update loaders warning #49633

GeoffreyBooth commented Sep 13, 2023 •

edited

Loading

nodejs-github-bot commented Sep 13, 2023

aduh95 commented Sep 13, 2023

GeoffreyBooth commented Sep 14, 2023 •

edited

Loading

nodejs-github-bot commented Sep 16, 2023

nodejs-github-bot commented Sep 18, 2023

nodejs-github-bot commented Sep 18, 2023

nodejs-github-bot commented Sep 18, 2023

guybedford left a comment

nodejs-github-bot commented Sep 19, 2023

esm: update loaders warning #49633

esm: update loaders warning #49633

Conversation

GeoffreyBooth commented Sep 13, 2023 • edited Loading

nodejs-github-bot commented Sep 13, 2023

aduh95 commented Sep 13, 2023

GeoffreyBooth commented Sep 14, 2023 • edited Loading

nodejs-github-bot commented Sep 16, 2023

nodejs-github-bot commented Sep 18, 2023

nodejs-github-bot commented Sep 18, 2023

nodejs-github-bot commented Sep 18, 2023

guybedford left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Sep 19, 2023

GeoffreyBooth commented Sep 13, 2023 •

edited

Loading

GeoffreyBooth commented Sep 14, 2023 •

edited

Loading