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.

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

Replace uses of deprecated V8 API #191

Closed
wants to merge 4,684 commits into from
Closed

Conversation

gahaas
Copy link

@gahaas gahaas commented Jun 21, 2024

Two fields on the v8::FastApiCallbackOptions struct were deprecated recently, fallback and wasm_memory. This PR removes uses of these two fields in node.js

This change is a refactoring and does not add new features. Therefore existing tests should be sufficient.

nodejs-github-bot and others added 30 commits May 10, 2024 18:03
PR-URL: nodejs#51657
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#51657
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#52453
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Remove calls of `add_argument_group()` where an existing argument group
is passed as an argument, this is deprecated since Python 3.11.

PR-URL: nodejs#52913
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#52820
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Harshitha K P <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Fixes: nodejs#46836
PR-URL: nodejs#48033
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
PR-URL: nodejs#51746
Refs: nodejs#51146
Refs: nodejs#50684
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
PR-URL: nodejs#44432
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#45722
Fixes: nodejs#43465
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
PR-URL: nodejs#51653
Fixes: nodejs#51482
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Add examples to `http` server.close, server.closeAllConnections,
server.closeIdleConnections. Also add notes about usage for both
server.close*Connections libraries.

PR-URL: nodejs#49091
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
PR-URL: nodejs#50308
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#52734
Reviewed-By: Daeyeon Jeong <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
PR-URL: nodejs#52932
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
PR-URL: nodejs#52924
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Signed-off-by: Matteo Collina <[email protected]>
PR-URL: nodejs#52940
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Spec mention stopImmediatePropagation should set both flags:
"stop propagation" and "stop immediate propagation".
So the second is not supported by Node.js as there is no
hierarchy and bubbling,
but the flags are both present as well as stopPropagation.
It would makes sense to follow specs on that.

Refs: https://dom.spec.whatwg.org/#dom-event-stopimmediatepropagation
PR-URL: nodejs#39463
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#38646
Refs: https://coverage.nodejs.org/coverage-16e00a15deafdad0/lib/readline.js.html#L441
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Added an example to the `execFileSync` method. This demonstrates how to
handle exceptions and access the stderr and stdio properties that are
attached to the `Error` object in a `catch` block.

Added a link to the detailed stdio section nested under
`child_process.spawn()` from each child_process sync method option
description.

Fixes: nodejs#39306
PR-URL: nodejs#39412
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#52283
Fixes: nodejs#52263
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Daeyeon Jeong <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Matthew Aitken <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#52827
Fixes: nodejs#52825
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Fix an issue where the autocomplete wrongly autocompletes
a value from a correct value to an undefined value when `node`
is built without an inspector by disabling the preview view.

fixes: nodejs#40635
PR-URL: nodejs#40661
Fixes: nodejs#40635
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Add test that confirms that
`stream.promises.pipeline(source, transform, dest, {end: false});`
only skips ending the destination stream.
`{end: false}` should still end any transform streams.

PR-URL: nodejs#48970
Reviewed-By: Luigi Pinca <[email protected]>
There is currently no documentation about what the `end` option in
`stream.promises.pipeline` does.

Refs: nodejs#40886
Refs: nodejs#34805 (comment)
Fixes: nodejs#45821
PR-URL: nodejs#48970
Reviewed-By: Luigi Pinca <[email protected]>
Fixes: nodejs#35339
Co-authored-by: Pooja D P <[email protected]>
Co-authored-by: Teutates <[email protected]>
Co-authored-by: Antoine du Hamel <[email protected]>
PR-URL: nodejs#43987
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
pthier and others added 29 commits June 3, 2024 13:39
Skip test-debugger-break and test-debugger-run-after-quit-restart to
unblock https://crrev.com/c/4290476
…#155)

The failing test is a regression test for nodejs#39717, which is about a crash that occurs when the flag is disabled. Given that that will no longer be possible once the flag is gone, this seems safe to disable forever.
# Conflicts:
#	test/parallel/parallel.status
# Conflicts:
#	test/parallel/parallel.status
Re-enable tests temporarily disabled
in a8952fc
Co-authored-by: Rezvan Mahdavi Hezaveh <[email protected]>
# Conflicts:
#	test/parallel/parallel.status
V8 will increase the maximum TypedArray size in
https://crrev.com/c/4872536; this CL adapts the tests to allow for that.

Tests that hard-code smaller sizes or are already tested in V8 are
removed; one test is adapted to not rely on a specific limit.
# Conflicts:
#	test/parallel/test-buffer-alloc.js
#	test/parallel/test-buffer-tostring-rangeerror.js
Use TypedArray::kMaxByteLength instead.
# Conflicts:
#	src/js_stream.cc
Error introduced when resolving merge-conflict.
The new callback should return v8::Intercepted::kYes/kNo to indicate
whether the operation was intercepted. This replaces the old approach
where the callback had to leave the return value unset or set it to
an empty handle to indicate that the the request wasn't intercepted.

See https://crrev.com/c/5465509 and https://crrev.com/c/5465513.
# Conflicts:
#	src/node_contextify.cc

# Conflicts:
#	src/node_contextify.cc
#	src/node_env_var.cc
* Add node_enable_deprecated_declarations_warnings GN flag

Warnings about using deprecated declarations were disabled by default
which made it hard to ensure that Node doesn't use V8's deprecated
Apis.

The flag allows enabling deprecated warnings and suppresses (hopefully)
known issues with using deprecated functionality in c-api.

The flag is off by default which preserves the existing behavior.

Drive-by: fix deps/openssl/unofficial.gni by exposing the required
OpenSSL compatibility level (OPENSSL_API_COMPAT) via public_configs.
Namely v8::FunctionCallbackInfo::Holder(), one should use This()
method instead.
See https://crrev.com/c/5444829 for details.
…8#183)

Previously is was defined via soon-to-be-deprecated
`v8::ObjectTemplate::SetAccessor(..)` which used to call setter even
for property stores via stream object.

The replacement V8 Api `v8::ObjectTemplate::SetNativeDataProperty(..)`
defines a properly behaving data property and thus a store to a stream
object will not trigger the "onread" setter callback.

In order to preserve the desired behavior of storing the value in the
receiver's internal field the "onread" property should be defined as
a proper JavaScript accessor.
# Conflicts:
#	src/base_object-inl.h
… (v8#189)

* Fix test-http-server-keepalive-req-gc

* Format

---------

Co-authored-by: Etienne Pierre-Doray <[email protected]>
Co-authored-by: Etienne Pierre-doray <[email protected]>
It's being deprecated and removed in V8.

PR-URL: nodejs#52910
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@gahaas gahaas closed this Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.