Skip to content

Conversation

@JakobJingleheimer
Copy link
Member

PR-URL: #55412
Reviewed-By: Matteo Collina [email protected]
Reviewed-By: Antoine du Hamel [email protected]

PR-URL: #55543
Reviewed-By: Jacob Smith [email protected]
Reviewed-By: Marco Ippolito [email protected]

PR-URL: #56382
Reviewed-By: Jordan Harband [email protected]
Reviewed-By: Geoffrey Booth [email protected]

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/loaders

@nodejs-github-botnodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. v22.x Issues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch. labels Jan 6, 2025
@JakobJingleheimer
Copy link
MemberAuthor

JakobJingleheimer commented Jan 12, 2025

Oh son of a…. Forgot that would break the PR.

@JakobJingleheimer
Copy link
MemberAuthor

@ruyadorno could this land soon before more conflicts arise?

aduh95and others added 21 commits January 23, 2025 11:00
PR-URL: nodejs#56707 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>
PR-URL: nodejs#55066 Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#56276 Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
V8 should be built with -fvisibility=hidden, otherwise the resulting binary would contain unnecessary symbols. In particular, on macOS, this leads to 5000+ weak symbols resolved at runtime, leading to a startup regression. On macOS this also reduces the binary size about ~10MB. It's only enabled on macOS in this patch as gcc can time out or run out of memory on some machines in the CI with -fvisibility=hidden. PR-URL: nodejs#56275Fixes: nodejs/performance#180 Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Daniel Lemire <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Marco Ippolito <[email protected]>
PR-URL: nodejs#56283 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#56295 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Xuguang Mei <[email protected]>
This reverts commit nodejs@bdb6d12. PR-URL: nodejs#56282 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Jason Zhang <[email protected]> Reviewed-By: Jake Yuesong Li <[email protected]>
PR-URL: nodejs#56282 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Jason Zhang <[email protected]> Reviewed-By: Jake Yuesong Li <[email protected]>
When the source map data are invalid json strings, skip construct `SourceMap` on it. Additionally, suppress exceptions on source map lookups and fix test runners crash on invalid source maps. PR-URL: nodejs#56299 Refs: nodejs#56296 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Xuguang Mei <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Pietro Marchini <[email protected]>
Only invalidates source map lookup cache when a new source map is found. This improves when user codes interleave with builtin functions, like `array.map`. PR-URL: nodejs#56299 Refs: nodejs#56296 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Xuguang Mei <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Pietro Marchini <[email protected]>
The issue is the same as other tests that time out. Refs: nodejs#54918 Refs: nodejs#54802 PR-URL: nodejs#56048 Reviewed-By: Gerhard Stöbich <[email protected]>
PR-URL: nodejs#56162 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Gabriel Schulhof <[email protected]>
We are in the process of updating macOS to version 13 in the Jenkins CI, but unfortunately this is taking longer than expected. Add it to the GitHub actions test matrix so that we have some coverage. Refs: nodejs/build#3686 PR-URL: nodejs#56307 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
`btoa` only supports latin-1 charset and produces invalid source mapping urls. PR-URL: nodejs#56315 Refs: nodejs#56296 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#56317 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
PR-URL: nodejs#56300 Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Xuguang Mei <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#54657 Refs: nodejs#54645 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Zeyu "Alex" Yang <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#56332 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
legendecasand others added 11 commits January 31, 2025 11:02
Fix the inspector_protocol/roll.py to fit node source directory structure. `roll.py` reads the `deps/v8/thrid_party/inspector_protocol/README.v8` to get the revision of the inspector_protocol that V8 depends on, and updates the local version to match. V8's copy of inspector_protocol modifies the namespace of `crdtp` library and does not export the symbols. So it can not be used outside of V8. PR-URL: nodejs#56649 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
Roll the inspector_protocol library to match V8's inspector_protocol revision. Update the node inspector to use the new `crdtp` protocol library. PR-URL: nodejs#56649 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
The `crdtp` library in the inspector_protocol is compiled as a library and linked to the node executable. PR-URL: nodejs#56649 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
test-global-setters.js was added in nodejs#26882 and hasn't been modified since. It appears that it avoids strict mode so that unscoped identifiers are treated as globals, but that's true in strict mode too. (In sloppy mode, an assignment to an undefined identifier will create a global, but that's not what this test does. In strict mode, those assignments will throw an error, which would seem to be what we would want.) PR-URL: nodejs#56742 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Guy Bedford <[email protected]>
By its own nature, the function returned by `common.mustSucceed()` cannot be used as a listener for `'error'` events. Write errors like `read ECONNRESET` or `write EPIPE`, should be ignored because the socket might be closed by the other peer while the request is sent. Refs: nodejs@3caa2c1a005652fdb3e8 PR-URL: nodejs#56756 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
Reduce chances of write errors while the request is sent. Refs: nodejs#56756 PR-URL: nodejs#56757 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Stefan Stojanovic <[email protected]>
Signed-off-by: Michael Dawson <[email protected]> PR-URL: nodejs#56592 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruy Adorno <[email protected]>
PR-URL: nodejs#56580 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>
PR-URL: nodejs#56259 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
PR-URL: nodejs#56795 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
PR-URL: nodejs#56318 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Akhil Marsonya <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
@aduh95aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 2, 2025
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 2, 2025
@nodejs-github-bot
Copy link
Collaborator

JakobJingleheimerand others added 3 commits February 3, 2025 11:41
PR-URL: nodejs#55412 Backport-PR-URL: nodejs#56494 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#55543 Backport-PR-URL: nodejs#56494 Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Marco Ippolito <[email protected]>
PR-URL: nodejs#56382 Backport-PR-URL: nodejs#56494 Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
@aduh95aduh95force-pushed the 22.x/findPackageJSON branch from b9754f4 to 4361c48CompareFebruary 3, 2025 10:43
@aduh95aduh95 requested a review from a team as a code ownerFebruary 3, 2025 10:43
@aduh95
Copy link
Contributor

aduh95 commented Feb 3, 2025

Landed in e0980fc...b341af0

@aduh95aduh95 closed this Feb 3, 2025
@JakobJingleheimerJakobJingleheimer deleted the 22.x/findPackageJSON branch February 3, 2025 10:49
aduh95 pushed a commit that referenced this pull request Feb 4, 2025
PR-URL: #55412 Backport-PR-URL: #56494 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
aduh95 added a commit that referenced this pull request Feb 4, 2025
PR-URL: #55543 Backport-PR-URL: #56494 Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Marco Ippolito <[email protected]>
aduh95 pushed a commit that referenced this pull request Feb 4, 2025
PR-URL: #56382 Backport-PR-URL: #56494 Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
aduh95 pushed a commit that referenced this pull request Feb 6, 2025
PR-URL: #55412 Backport-PR-URL: #56494 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
aduh95 added a commit that referenced this pull request Feb 6, 2025
PR-URL: #55543 Backport-PR-URL: #56494 Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Marco Ippolito <[email protected]>
aduh95 pushed a commit that referenced this pull request Feb 6, 2025
PR-URL: #56382 Backport-PR-URL: #56494 Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lib / srcIssues and PRs related to general changes in the lib or src directory.needs-ciPRs that need a full CI run.v22.xIssues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

20 participants

@JakobJingleheimer@nodejs-github-bot@aduh95@ruyadorno@marco-ippolito@NathanBaulch@MoLow@joyeecheung@origranot@lpinca@legendecas@jazelly@vmoroz@targos@zcbenz@yiyunlei@mhdawson@kevinuehara@IlyasShabi@mcollina