chore: upgrade to Node.js v18 by codebytere · Pull Request #35999 · electron/electron

codebytere · 2022-10-12T08:48:41Z

Description of Change

Upgrade to Node.js v18.

Checklist

PR description included and stakeholders cc'd
npm test passes
PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: Upgraded Node.js to v18.10.0

nornagon · 2022-10-27T18:26:43Z

+rename from deps/base64/base64/lib/arch/avx/codec.c
+rename to deps/base64/base64/lib/arch/avx/avx_codec.c


huh, how come we needed to rename these?

gn doesn't seem to allow duplicate filenames even if they're in different directories - this is what happens otherwise:

electron on git:upgrade-nodejs-v18 ❯ e build 10:12AM Running "e load-xcode --quiet" Running "python3 goma_auth.py info" in /Users/codebytere/.electron_build_tools/third_party/goma Running "ninja -j 200 electron" in /Users/codebytere/Developer/electron-gn/src/out/Testing [0/1] Regenerating ninja files ERROR at //third_party/electron_node/deps/base64/BUILD.gn:10:1: Duplicate object file static_library("base64") { ^------------------------- The target //third_party/electron_node/deps/base64:base64 generates two object files with the same name: obj/third_party/electron_node/deps/base64/base64/codec.o It could be you accidentally have a file listed twice in the sources. Or, depending on how your toolchain maps sources to object files, two source files with the same name in different directories could map to the same object file. In the latter case, either rename one of the files or move one of the sources to a separate source_set to avoid them both being in the same target. FAILED: build.ninja.stamp

Generally the way to solve this in GN is to use separate targets. Make an base64_avx target or something that has the avx-specific files in it. object names only need to be unique within a single target.

nornagon · 2022-10-27T18:33:42Z

  "parallel/test-trace-events-v8",
  "parallel/test-trace-events-vm",
  "parallel/test-trace-events-worker-metadata",
+  "parallel/test-wasm-web-api",


what breaks here?

@nornagon i can add a follow-up for this but what's breaking would require a refactor far (imo) beyond the scope of this PR. Essentially, WebAssembly.compileStreaming and WebAssembly.instantiateStreaming are undefined when they shouldn't be. That's happening because Node.js' logic for enabling them here is called via node::SetIsolateUpForNode, which by necessity can only be called on an existing isolate. However, the switch to enable it here is called by gin during its own isolate initialization, which means that it ends up being undefined. I can look into a good way to fix this but imo impact is low and refactor burden is high so i'd prefer to just disable it for now.

Opened #36282

Fair enough, happy to leave this for a followup, but it should probably be a release blocker as those methods are expected to be available in this version of Node.

bootstrap: implement run-time user-land snapshots via --build-snapshot and --snapshot-blob

nodejs/node#42701

nodejs/node#39987

nornagon

LGTM but i'd love to see a refactor to use a separate GN target instead of renaming files!

jkleinsc

API LGTM

release-clerk · 2022-11-10T21:31:24Z

Release Notes Persisted

Upgraded Node.js to v18.10.0

treysis · 2023-01-26T20:39:55Z

 import { defer, delay } from './spec-helpers';

+// See https://github.com/nodejs/node/issues/40702.
+dns.setDefaultResultOrder('ipv4first');


This "fix" should be reverted. It causes problems for lots of people using IPv6.

Better use net.autoSelectFamily instead which enabled Happy Eyeballs according to RFC 8305 (requires upgrading to node v18.13).

Also why is this change not addressed in the release notes in this PR?

This change is not in the upstream and some downstream projects were expecting this option to change as per v17 release notes.

If there is a problem with it, it should raised as an issue of this project, discussed with the community and fixed accordingly. Not by silently hiding it in a large PR.

Fully agree. This has to be reverted.

* chore: update to Node.js v18 * child_process: improve argument validation nodejs/node#41305 * bootstrap: support configure-time user-land snapshot nodejs/node#42466 * chore: update GN patch * src: disambiguate terms used to refer to builtins and addons nodejs/node#44135 * src: use a typed array internally for process._exiting nodejs/node#43883 * chore: lib/internal/bootstrap -> lib/internal/process * src: disambiguate terms used to refer to builtins and addons nodejs/node#44135 * chore: remove redudant browserGlobals patch * chore: update BoringSSL patch * src: allow embedder-provided PageAllocator in NodePlatform nodejs/node#38362 * chore: fixup Node.js crypto tests - nodejs/node#44171 - nodejs/node#41600 * lib: add Promise methods to avoid-prototype-pollution lint rule nodejs/node#43849 * deps: update V8 to 10.1 nodejs/node#42657 * src: add kNoBrowserGlobals flag for Environment nodejs/node#40532 * chore: consolidate asar initialization patches * deps: update V8 to 10.1 nodejs/node#42657 * deps: update V8 to 9.8 nodejs/node#41610 * src,crypto: remove AllocatedBuffers from crypto_spkac nodejs/node#40752 * build: enable V8's shared read-only heap nodejs/node#42809 * src: fix ssize_t error from nghttp2.h nodejs/node#44393 * chore: fixup ESM patch * chore: fixup patch indices * src: merge NativeModuleEnv into NativeModuleLoader nodejs/node#43824 * [API] Pass OOMDetails to OOMErrorCallback https://chromium-review.googlesource.com/c/v8/v8/+/3647827 * src: iwyu in cleanup_queue.cc * src: return Maybe from a couple of functions nodejs/node#39603 * src: clean up embedder API nodejs/node#35897 * src: refactor DH groups to delete crypto_groups.h nodejs/node#43896 * deps,src: use SIMD for normal base64 encoding nodejs/node#39775 * chore: remove deleted source file * chore: update patches * chore: remove deleted source file * lib: add fetch nodejs/node#41749 * chore: remove nonexistent node specs * test: split report OOM tests nodejs/node#44389 * src: trace fs async api nodejs/node#44057 * http: trace http request / response nodejs/node#44102 * test: split test-crypto-dh.js nodejs/node#40451 * crypto: introduce X509Certificate API nodejs/node#36804 * src: split property helpers from node::Environment nodejs/node#44056 * nodejs/node#38905 bootstrap: implement run-time user-land snapshots via --build-snapshot and --snapshot-blob * lib,src: implement WebAssembly Web API nodejs/node#42701 * fixup! deps,src: use SIMD for normal base64 encoding * fixup! src: refactor DH groups to delete crypto_groups.h * chore: fixup base64 GN file * fix: check that node::InitializeContext() returns true * chore: delete _noBrowserGlobals usage * chore: disable fetch in renderer procceses * dns: default to verbatim=true in dns.lookup() nodejs/node#39987 Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>

electron-cation Bot added the new-pr 🌱 PR opened recently label Oct 12, 2022

codebytere force-pushed the upgrade-nodejs-v18 branch 5 times, most recently from 3621500 to fe16af8 Compare October 13, 2022 15:50

codebytere added semver/minor backwards-compatible functionality semver/major incompatible API changes labels Oct 14, 2022

electron-cation Bot added the api-review/requested 🗳 label Oct 14, 2022

codebytere removed semver/minor backwards-compatible functionality api-review/requested 🗳 labels Oct 14, 2022

electron-cation Bot added the api-review/requested 🗳 label Oct 14, 2022

codebytere force-pushed the upgrade-nodejs-v18 branch from 97067b8 to 0ad2240 Compare October 17, 2022 12:12

electron-cation Bot removed the new-pr 🌱 PR opened recently label Oct 19, 2022

codebytere force-pushed the upgrade-nodejs-v18 branch 2 times, most recently from f85e532 to a9fdc59 Compare October 19, 2022 15:03

codebytere mentioned this pull request Oct 24, 2022

Enable userland snapshotting in Node.js #36118

Open

codebytere force-pushed the upgrade-nodejs-v18 branch 2 times, most recently from 88c4b58 to 023e065 Compare October 24, 2022 15:01

codebytere marked this pull request as ready for review October 25, 2022 06:16

codebytere requested review from a team as code owners October 25, 2022 06:16

codebytere marked this pull request as draft October 25, 2022 06:16

codebytere force-pushed the upgrade-nodejs-v18 branch 2 times, most recently from 62c1a30 to 9a98ccb Compare October 26, 2022 19:47

codebytere marked this pull request as ready for review October 26, 2022 21:03

nornagon reviewed Oct 27, 2022

View reviewed changes

deepak1556 reviewed Oct 29, 2022

View reviewed changes

Comment thread shell/common/node_bindings.cc Outdated

deepak1556 reviewed Oct 29, 2022

View reviewed changes

Comment thread shell/common/node_bindings.cc

deepak1556 reviewed Oct 29, 2022

View reviewed changes

Comment thread patches/node/build_add_gn_build_files.patch Outdated

codebytere added 14 commits November 8, 2022 19:52

src: trace fs async api

6121335

nodejs/node#44057

http: trace http request / response

ae2fb5c

nodejs/node#44102

test: split test-crypto-dh.js

d304b49

nodejs/node#40451

crypto: introduce X509Certificate API

f9634a6

nodejs/node#36804

src: split property helpers from node::Environment

fd70677

nodejs/node#44056

https://github.com/nodejs/node/pull/38905

fab321e

bootstrap: implement run-time user-land snapshots via --build-snapshot and --snapshot-blob

lib,src: implement WebAssembly Web API

41d7d26

nodejs/node#42701

fixup! deps,src: use SIMD for normal base64 encoding

68c9203

fixup! src: refactor DH groups to delete crypto_groups.h

7ebcc5a

chore: fixup base64 GN file

a7e677d

fix: check that node::InitializeContext() returns true

24fc9aa

chore: delete _noBrowserGlobals usage

d97c41f

chore: disable fetch in renderer procceses

b23a003

dns: default to verbatim=true in dns.lookup()

fe748fd

nodejs/node#39987

codebytere force-pushed the upgrade-nodejs-v18 branch from 018957a to fe748fd Compare November 8, 2022 18:52

nornagon approved these changes Nov 9, 2022

View reviewed changes

VerteDinde approved these changes Nov 9, 2022

View reviewed changes

VerteDinde added the no-backport label Nov 9, 2022

jkleinsc approved these changes Nov 10, 2022

View reviewed changes

codebytere merged commit 75d2caf into main Nov 10, 2022

codebytere deleted the upgrade-nodejs-v18 branch November 10, 2022 21:31

jkleinsc mentioned this pull request Nov 14, 2022

chore: bump node to v18.12.1 (main) #36019

Closed

treysis reviewed Jan 26, 2023

View reviewed changes

treysis mentioned this pull request Jan 27, 2023

feat: activate Happy Eyeballs in NodeJS #37045

Closed

2 tasks

commit-lint Bot mentioned this pull request Feb 24, 2023

[pull] main from electron:main Blaquez-home/electron#3

Open

codebytere mentioned this pull request Jun 13, 2023

fix: window.open causing occasional Node.js crashes #38754

Merged

4 tasks

Clem-Fern mentioned this pull request Aug 18, 2023

SSH: unable to connect to ssh server using kex algorithm diffie-hellman-group1-sha1 Eugeny/tabby#8840

Closed

laurent22 mentioned this pull request Nov 11, 2023

Webdav sync is broken since 2.12.15 laurent22/joplin#8788

Closed

		rename from deps/base64/base64/lib/arch/avx/codec.c
		rename to deps/base64/base64/lib/arch/avx/avx_codec.c

Uh oh!

Conversation

codebytere commented Oct 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of Change

Checklist

Release Notes

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nornagon left a comment • edited by codebytere Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkleinsc left a comment

Choose a reason for hiding this comment

Uh oh!

release-clerk Bot commented Nov 10, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

treysis Jan 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miyurusankalpa Jan 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

codebytere commented Oct 12, 2022 •

edited

Loading

nornagon left a comment •

edited by codebytere

Loading

treysis Jan 27, 2023 •

edited

Loading

miyurusankalpa Jan 27, 2023 •

edited

Loading