Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[v19.x backport] esm: move hooks handling into separate class #46511

Closed
wants to merge 131 commits into from
Closed

[v19.x backport] esm: move hooks handling into separate class #46511

wants to merge 131 commits into from

Conversation

GeoffreyBooth
Copy link
Member

PR-URL: #45869
Reviewed-By: Antoine du Hamel duhamelantoine1995@gmail.com
Reviewed-By: Jacob Smith jacob@frende.me
Reviewed-By: Rich Trott rtrott@gmail.com
Reviewed-By: Matteo Collina matteo.collina@gmail.com

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/loaders
  • @nodejs/modules

@nodejs-github-bot nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem. v19.x labels Feb 6, 2023
GeoffreyBooth added a commit to GeoffreyBooth/node that referenced this pull request Feb 6, 2023
@GeoffreyBooth
esm: move hooks handling into separate class
185956b 
PR-URL: nodejs#45869
Backport-PR-URL: nodejs#46511
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@GeoffreyBooth GeoffreyBooth mentioned this pull request Feb 6, 2023
Merged
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

targos
targos approved these changes Feb 6, 2023
View reviewed changes
Copy link
Member

@targos targos left a comment

Choose a reason for hiding this comment

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

rubberstamp

@GeoffreyBooth GeoffreyBooth added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed needs-ci PRs that need a full CI run. labels Feb 6, 2023
RafaelGSS and others added 17 commits March 13, 2023 15:16
@RafaelGSS @targos
src,process: add permission model
9a09e37 
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
PR-URL: nodejs#44004
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@nodejs-github-bot @targos
tools: update lint-md-dependencies to rollup@3.17.2
f1a237c 
PR-URL: nodejs#46771
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Tierney Cyren <hello@bnb.im>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@panva @targos
lib: fix DOMException property descriptors after being lazy loaded
812c84b 
PR-URL: nodejs#46799
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@nodejs-github-bot @targos
deps: update simdutf to 3.2.1
fa8e38f 
PR-URL: nodejs#46800
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@panva @targos
test: remove useless WPT init scripts
9344e93 
PR-URL: nodejs#46801
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@panva @targos
test: update encoding WPTs
a3cabdb 
PR-URL: nodejs#46802
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@panva @targos
tools: upload daily WPT Report to both staging and production
457cb35 
PR-URL: nodejs#46803
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Tierney Cyren <hello@bnb.im>
Reviewed-By: James M Snell <jasnell@gmail.com>
@panva @targos
test: fix WPT title when no META title is present
977fcc4 
PR-URL: nodejs#46804
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
@tniessen @targos
test: simplify test-tls-alert
4a810dc 
Avoid the process 'exit' event handler and use execFile instead of
manual stream operations.

Refs: nodejs#46751
PR-URL: nodejs#46805
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@batrla @targos
test: fix test-net-connect-reset-until-connected
10486a0 
Fixes: nodejs#43446
PR-URL: nodejs#46781
Reviewed-By: theanarkh <theratliter@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@nodejs-github-bot @targos
tools: update lint-md-dependencies to rollup@3.17.3
563b243 
PR-URL: nodejs#46843
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@debadree25 @targos
benchmark: add benchmark for EventTarget add and remove
171b2a8 
Refs: nodejs/performance#60
PR-URL: nodejs#46779
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@RaisinTen @targos
doc: add steps about signing the binary in single-executable docs
33b5c3e 
We didn't catch this in nodejs#45038
because the binary wasn't signed by default unlike the official Node.js
binary, which is signed by the Node.js Foundation identity by default.

Refs: nodejs/postject#76 (macOS arm64 part only)
Fixes: nodejs/postject#75
Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: nodejs#46764
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@aduh95 @targos
lib: add trailing commas to more internal files
8fb19f7 
PR-URL: nodejs#46811
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@addaleax @targos
test: fix os-release check for Ubuntu in SEA test
822b008 
For example, my `/etc/os-release` file begins with

```
PRETTY_NAME="Ubuntu 22.04.2 LTS"
NAME="Ubuntu"
VERSION_ID="22.04"
```

so in order to match the regexp here, the `/m` flag is necessary.

PR-URL: nodejs#46838
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@nodejs-github-bot @targos
deps: update corepack to 0.17.0
5921af7 
PR-URL: nodejs#46842
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
@mcollina @targos
stream: always delay construct callback by a nextTick
b7bb4e3 
Signed-off-by: Matteo Collina <hello@matteocollina.com>
Fixes: nodejs#46765
PR-URL: nodejs#46818
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
tniessen and others added 21 commits March 13, 2023 15:59
@tniessen @targos
tls: support automatic DHE
119b241 
Node.js has so far only supported user-defined DHE parameters and even
recommended generating custom parameters. This change lets users set the
dhparam option to 'auto' instead, in which case DHE parameters of
sufficient strength are selected automatically (from a small set of
well-known parameters). This has been recommended by OpenSSL for quite a
while, and it makes it much easier for Node.js TLS servers to properly
support DHE-based perfect forward secrecy.

This also updates the documentation to prioritize ECDHE over DHE, mostly
because the former tends to be more efficient and is enabled by default.

PR-URL: nodejs#46978
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@gireeshpunathil @targos
doc: amend support tier qualifier
e5ffba0 
Amend Tier 3 category definition to remove a
tentative clause and introduce a more concrete one.

Refs: nodejs#42802
PR-URL: nodejs#42805
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
@Dailyscat @targos
debugger: improve validations and documents for watch and unwatch
3a24f7b 
- debugger: add string validation for watch(expr)
- debugger: add help document for watch(index)
- test: add test for watch(index) command
- doc: add information on unwatch(index) option

PR-URL: nodejs#46947
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
@debadree25 @targos
benchmark: add a benchmark for URLSearchParams creation and toString()
cf80847 
Refs: nodejs/performance#56
PR-URL: nodejs#46810
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@vhiairrassary @targos
doc: fix typo in test.md
1e8fe6f 
PR-URL: nodejs#47053
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Qingyu Deng <i@ayase-lab.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@MoLow @targos
test: fix flakyness in test-runner reporter test
ef5ce68 
PR-URL: nodejs#45930
Fixes: nodejs#45929
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@MoLow @targos
test_runner: flatten TAP output when running using --test
aa35ec7 
PR-URL: nodejs#46440
Fixes: nodejs#45833
Refs: nodejs#45833
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@richiemccoll @targos
test_runner: emit test-only diagnostic warning
62d0c0f 
PR-URL: nodejs#46540
Fixes: nodejs#46448
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MoLow @targos
test_runner: bootstrap reporters before running tests
3641388 
PR-URL: nodejs#46737
Fixes: nodejs#46747
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@aduh95 @targos
events: add trailing commas in source files
33ce360 
PR-URL: nodejs#46759
Reviewed-By: Deokjin Kim <deokjin81.kim@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
@tniessen @targos
lib,src: fix a few typos in comments
1a97c4b 
PR-URL: nodejs#46835
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
@MoLow @targos
test_runner: reset count on watch mode
8a55ef4 
PR-URL: nodejs#46577
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@aduh95 @targos
lib: add trailing commas to all public core modules
0b7d2a4 
PR-URL: nodejs#46848
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
@anonrig @targos
url: offload URLSearchParams initialization
1674601 
PR-URL: nodejs#46867
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
@MoLow @targos
test_runner: fix reconstruction of errors extracted from YAML
63cab16 
PR-URL: nodejs#46872
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Nitzan Uziely <linkgoron@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MoLow @targos
test_runner: avoid running twice tests in describe
95bf734 
PR-URL: nodejs#46888
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@aduh95 @targos
lib: enforce use of trailing commas
2ea710d 
PR-URL: nodejs#46881
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
@cjihrig @targos
test_runner: track bootstrapping process
4b0f81d 
This commit updates the test harness and root test to track
when bootstrapping has completed.

PR-URL: nodejs#46962
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@cjihrig @targos
test_runner: throw if harness is not bootstrapped
5210b78 
This commit updates the test harness to re-throw uncaught errors
if bootstrapping has not completed. This updates the existing
logic which tried to detect a specific error code.

PR-URL: nodejs#46962
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@cjihrig @targos
test_runner: handle errors not bound to tests
a14cf7d 
This commit addresses a previously untested branch of the code.
It is possible when using the test runner that an error occurs
outside of a test. In this case, the test runner would simply
rethrow the error. This commit updates the logic to handle the
error in the same fashion as other uncaughtExceptions.

PR-URL: nodejs#46962
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@MoLow @targos
test_runner: default to spec reporter when on TTY environment
4122a4d 
PR-URL: nodejs#46969
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@targos
Copy link
Member

targos commented Mar 13, 2023

It looks like this PR was forgotten. @GeoffreyBooth can you update it so I include it in tomorrow's release?

@GeoffreyBooth
esm: move hooks handling into separate class
1e8c7af 
PR-URL: nodejs#45869
Backport-PR-URL: nodejs#46511
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@GeoffreyBooth
Copy link
Member Author

Quickly rebased. Let’s please stop backporting lint/style change PRs that cause conflicts in pending PRs, at least until all the pending PRs are merged in first. It just causes tons of conflicts for little to no gain, and resolving all those conflicts might introduce bugs. Lint/style changes are the lowest priority; they can be backported dead last after everything else they might otherwise introduce conflicts into. cc @aduh95

targos pushed a commit that referenced this pull request Mar 14, 2023
@GeoffreyBooth @targos
esm: move hooks handling into separate class
5eb5be8 
PR-URL: #45869
Backport-PR-URL: #46511
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@targos
Copy link
Member

targos commented Mar 14, 2023

Thanks. Landed in 5eb5be8

@targos targos closed this Mar 14, 2023
@GeoffreyBooth GeoffreyBooth deleted the backport-hooks-class branch March 14, 2023 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@targos targos targos approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. esm Issues and PRs related to the ECMAScript Modules implementation. process Issues and PRs related to the process subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.