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

src: use v8::Isolate::GetDefaultLocale() to compute navigator.language #54279

Closed
wants to merge 3 commits into from
Closed

src: use v8::Isolate::GetDefaultLocale() to compute navigator.language #54279

wants to merge 3 commits into from
+45 −10

Conversation

joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Aug 9, 2024 •
edited
Loading

src: use v8::Isolate::GetDefaultLocale() to compute navigator.language

Using the Intl API to get the default locale slows down the startup
significantly. This patch uses a new v8 API to get the default locale
directly.

Refs: #53765 (comment)

First commit comes from https://chromium-review.googlesource.com/c/v8/v8/+/5772938

From benchmark CI:

https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1594/console

14:54:15                                                                                           confidence improvement accuracy (*)   (**)  (***)
14:54:15 misc/startup-core.js count=30 mode='process' script='benchmark/fixtures/require-builtins'        ***     18.60 %       ±0.30% ±0.41% ±0.54%
14:54:15 misc/startup-core.js count=30 mode='process' script='test/fixtures/semicolon'                    ***     25.02 %       ±2.92% ±3.93% ±5.19%
14:54:15 misc/startup-core.js count=30 mode='process' script='test/fixtures/snapshot/typescript'          ***      4.32 %       ±0.04% ±0.06% ±0.07%
14:54:15 misc/startup-core.js count=30 mode='worker' script='benchmark/fixtures/require-builtins'                 -0.05 %       ±0.18% ±0.24% ±0.31%
14:54:15 misc/startup-core.js count=30 mode='worker' script='test/fixtures/semicolon'                              0.02 %       ±0.36% ±0.47% ±0.62%
14:54:15 misc/startup-core.js count=30 mode='worker' script='test/fixtures/snapshot/typescript'            **     -1.48 %       ±0.87% ±1.16%

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/security-wg
  • @nodejs/v8-update
  • @nodejs/web-standards

@nodejs-github-bot nodejs-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. labels Aug 9, 2024
ljharb
ljharb approved these changes Aug 9, 2024
View reviewed changes
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

thanks, this was beyond my skills but definitely seems like an improvement :-)

@RedYetiDev RedYetiDev added wip Issues and PRs that are still a work in progress. v8 engine Issues and PRs related to the V8 dependency. dependencies Pull requests that update a dependency file. labels Aug 9, 2024
@joyeecheung
Copy link
Member Author

From benchmark CI:

https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1594/console

14:54:15                                                                                           confidence improvement accuracy (*)   (**)  (***)
14:54:15 misc/startup-core.js count=30 mode='process' script='benchmark/fixtures/require-builtins'        ***     18.60 %       ±0.30% ±0.41% ±0.54%
14:54:15 misc/startup-core.js count=30 mode='process' script='test/fixtures/semicolon'                    ***     25.02 %       ±2.92% ±3.93% ±5.19%
14:54:15 misc/startup-core.js count=30 mode='process' script='test/fixtures/snapshot/typescript'          ***      4.32 %       ±0.04% ±0.06% ±0.07%
14:54:15 misc/startup-core.js count=30 mode='worker' script='benchmark/fixtures/require-builtins'                 -0.05 %       ±0.18% ±0.24% ±0.31%
14:54:15 misc/startup-core.js count=30 mode='worker' script='test/fixtures/semicolon'                              0.02 %       ±0.36% ±0.47% ±0.62%
14:54:15 misc/startup-core.js count=30 mode='worker' script='test/fixtures/snapshot/typescript'            **     -1.48 %       ±0.87% ±1.16%

hubot pushed a commit to v8/v8 that referenced this pull request Aug 16, 2024
@joyeecheung
[api] add v8::Isolate::GetDefaultLocale()
e74d0f4 
This allows embedders to query the default locale used by Intl
APIs.

This information is already available to user land via
Intl?.Collator().resolvedOptions().locale, the issue with this
is that it's a lot slower and requires dynamic access to Intl
API, which is subject to patching and prototype pollution so it's
not as reliable for embedders than having a V8 API to query
the default locale directly.

Refs: nodejs/node#54279
Change-Id: I5a1823993c9ae79f8f61f54c6464daf882a09ba3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5772938
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Joyee Cheung <joyee@igalia.com>
Cr-Commit-Position: refs/heads/main@{#95678}
@joyeecheung
deps: V8: cherry-pick e74d0f437fcd
238d92a 
Original commit message:

    [api] add v8::Isolate::GetDefaultLocale()

    This allows embedders to query the default locale used by Intl
    APIs.

    This information is already available to user land via
    Intl?.Collator().resolvedOptions().locale, the issue with this
    is that it's a lot slower and requires dynamic access to Intl
    API, which is subject to patching and prototype pollution so it's
    not as reliable for embedders than having a V8 API to query
    the default locale directly.

    Refs: nodejs#54279
    Change-Id: I5a1823993c9ae79f8f61f54c6464daf882a09ba3
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5772938
    Reviewed-by: Leszek Swirski <leszeks@chromium.org>
    Commit-Queue: Joyee Cheung <joyee@igalia.com>
    Cr-Commit-Position: refs/heads/main@{#95678}

Refs: v8/v8@e74d0f4
@joyeecheung
src: use v8::Isolate::GetDefaultLocale() to compute navigator.language
4857714 
Using the Intl API to get the default locale slows down the startup
significantly. This patch uses a new v8 API to get the default locale
directly.
@joyeecheung joyeecheung marked this pull request as ready for review August 19, 2024 16:52
@joyeecheung joyeecheung changed the title [wip] src: use v8::Isolate::GetDefaultLocale() to compute navigator.language src: use v8::Isolate::GetDefaultLocale() to compute navigator.language Aug 19, 2024
@joyeecheung joyeecheung added request-ci Add this label to start a Jenkins CI on a PR. and removed wip Issues and PRs that are still a work in progress. labels Aug 19, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 19, 2024
@joyeecheung
Copy link
Member Author

Upstream CL has landed, backported and this should be ready for review now.

cc @nodejs/startup

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/6142/

@nodejs-github-bot
Copy link
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-s390x,v8test=v8test/6142/

@nodejs-github-bot
Copy link
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-ppc64le,v8test=v8test/6142/

@joyeecheung joyeecheung added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Aug 19, 2024
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/6143/

@nodejs-github-bot
Copy link
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-s390x,v8test=v8test/6143/

@nodejs-github-bot
Copy link
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-ppc64le,v8test=v8test/6143/

@codecov Codecov
Copy link

codecov bot commented Aug 19, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 95.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 87.31%. Comparing base (2c14615) to head (c330418).
Report is 403 commits behind head on main.

Files with missing lines Patch % Lines
src/node_config.cc 93.75% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #54279      +/-   ##
==========================================
+ Coverage   87.08%   87.31%   +0.23%     
==========================================
  Files         648      648              
  Lines      182341   182376      +35     
  Branches    34982    34982              
==========================================
+ Hits       158783   159240     +457     
+ Misses      16831    16401     -430     
- Partials     6727     6735       +8
Files with missing lines Coverage Δ
lib/internal/navigator.js 99.32% <100.00%> (+<0.01%) ⬆️
lib/internal/process/pre_execution.js 88.46% <ø> (-2.46%) ⬇️
src/node_external_reference.h 100.00% <ø> (ø)
src/node_config.cc 96.42% <93.75%> (-3.58%) ⬇️

... and 62 files with indirect coverage changes

bnoordhuis
bnoordhuis approved these changes Aug 19, 2024
View reviewed changes
lib/internal/navigator.js Show resolved Hide resolved
@nodejs-github-bot
Copy link
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/6155/

@nodejs-github-bot
Copy link
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-s390x,v8test=v8test/6155/

@nodejs-github-bot
Copy link
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-ppc64le,v8test=v8test/6155/

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@joyeecheung joyeecheung added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. labels Aug 29, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 29, 2024
@nodejs-github-bot
Copy link
Collaborator

Landed in c44d41b...75741a1

nodejs-github-bot pushed a commit that referenced this pull request Aug 29, 2024
@joyeecheung @nodejs-github-bot
deps: V8: cherry-pick e74d0f437fcd
54e20c1 
Original commit message:

    [api] add v8::Isolate::GetDefaultLocale()

    This allows embedders to query the default locale used by Intl
    APIs.

    This information is already available to user land via
    Intl?.Collator().resolvedOptions().locale, the issue with this
    is that it's a lot slower and requires dynamic access to Intl
    API, which is subject to patching and prototype pollution so it's
    not as reliable for embedders than having a V8 API to query
    the default locale directly.

    Refs: #54279
    Change-Id: I5a1823993c9ae79f8f61f54c6464daf882a09ba3
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5772938
    Reviewed-by: Leszek Swirski <leszeks@chromium.org>
    Commit-Queue: Joyee Cheung <joyee@igalia.com>
    Cr-Commit-Position: refs/heads/main@{#95678}

Refs: v8/v8@e74d0f4
PR-URL: #54279
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs-github-bot pushed a commit that referenced this pull request Aug 29, 2024
@joyeecheung @nodejs-github-bot
src: use v8::Isolate::GetDefaultLocale() to compute navigator.language
75741a1 
Using the Intl API to get the default locale slows down the startup
significantly. This patch uses a new v8 API to get the default locale
directly.

PR-URL: #54279
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Aug 30, 2024
@joyeecheung @RafaelGSS
deps: V8: cherry-pick e74d0f437fcd
e75b14b 
Original commit message:

    [api] add v8::Isolate::GetDefaultLocale()

    This allows embedders to query the default locale used by Intl
    APIs.

    This information is already available to user land via
    Intl?.Collator().resolvedOptions().locale, the issue with this
    is that it's a lot slower and requires dynamic access to Intl
    API, which is subject to patching and prototype pollution so it's
    not as reliable for embedders than having a V8 API to query
    the default locale directly.

    Refs: #54279
    Change-Id: I5a1823993c9ae79f8f61f54c6464daf882a09ba3
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5772938
    Reviewed-by: Leszek Swirski <leszeks@chromium.org>
    Commit-Queue: Joyee Cheung <joyee@igalia.com>
    Cr-Commit-Position: refs/heads/main@{#95678}

Refs: v8/v8@e74d0f4
PR-URL: #54279
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Aug 30, 2024
@joyeecheung @RafaelGSS
src: use v8::Isolate::GetDefaultLocale() to compute navigator.language
f1ce3b1 
Using the Intl API to get the default locale slows down the startup
significantly. This patch uses a new v8 API to get the default locale
directly.

PR-URL: #54279
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Aug 30, 2024
@joyeecheung @RafaelGSS
deps: V8: cherry-pick e74d0f437fcd
87acad9 
Original commit message:

    [api] add v8::Isolate::GetDefaultLocale()

    This allows embedders to query the default locale used by Intl
    APIs.

    This information is already available to user land via
    Intl?.Collator().resolvedOptions().locale, the issue with this
    is that it's a lot slower and requires dynamic access to Intl
    API, which is subject to patching and prototype pollution so it's
    not as reliable for embedders than having a V8 API to query
    the default locale directly.

    Refs: #54279
    Change-Id: I5a1823993c9ae79f8f61f54c6464daf882a09ba3
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5772938
    Reviewed-by: Leszek Swirski <leszeks@chromium.org>
    Commit-Queue: Joyee Cheung <joyee@igalia.com>
    Cr-Commit-Position: refs/heads/main@{#95678}

Refs: v8/v8@e74d0f4
PR-URL: #54279
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Aug 30, 2024
@joyeecheung @RafaelGSS
src: use v8::Isolate::GetDefaultLocale() to compute navigator.language
bd61a64 
Using the Intl API to get the default locale slows down the startup
significantly. This patch uses a new v8 API to get the default locale
directly.

PR-URL: #54279
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Aug 30, 2024
@joyeecheung @RafaelGSS
deps: V8: cherry-pick e74d0f437fcd
b00c087 
Original commit message:

    [api] add v8::Isolate::GetDefaultLocale()

    This allows embedders to query the default locale used by Intl
    APIs.

    This information is already available to user land via
    Intl?.Collator().resolvedOptions().locale, the issue with this
    is that it's a lot slower and requires dynamic access to Intl
    API, which is subject to patching and prototype pollution so it's
    not as reliable for embedders than having a V8 API to query
    the default locale directly.

    Refs: #54279
    Change-Id: I5a1823993c9ae79f8f61f54c6464daf882a09ba3
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5772938
    Reviewed-by: Leszek Swirski <leszeks@chromium.org>
    Commit-Queue: Joyee Cheung <joyee@igalia.com>
    Cr-Commit-Position: refs/heads/main@{#95678}

Refs: v8/v8@e74d0f4
PR-URL: #54279
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Aug 30, 2024
@joyeecheung @RafaelGSS
src: use v8::Isolate::GetDefaultLocale() to compute navigator.language
f8cbbc6 
Using the Intl API to get the default locale slows down the startup
significantly. This patch uses a new v8 API to get the default locale
directly.

PR-URL: #54279
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Aug 30, 2024
Merged
@bnoordhuis
Copy link
Member

For future reference: this PR didn't bump v8_embedder_string. v22.7.0, v22.8.0 and v22.9.0 all report the same V8 version but v22.7.0 does not have the new API.

bnoordhuis added a commit to bnoordhuis/libv8-node that referenced this pull request Sep 18, 2024
@bnoordhuis
Update to node 22.9.0
f44c672 
Note the V8 version didn't change between 22.7.0 and 22.9.0 although
Node.js did add a new API in nodejs/node#54279

Remove unused and/or unnecessary patches. Upstream disabled Maglev so
we no longer have to do that manually. The genv8constants.py patches
were for a script that I removed upstream last year.
@bnoordhuis bnoordhuis mentioned this pull request Sep 18, 2024
Merged
@joyeecheung
Copy link
Member Author

hmm, the PR diff did bump the string (https://github.com/nodejs/node/pull/54279/files#diff-9513a0c08584f4800e1461ca8c3456d1aeb899968ea14b2f6859ea462c37b5bb), but I think it might have been lost during merging if another PR that bumped the string landed before it.

@joyeecheung
Copy link
Member Author

joyeecheung commented Sep 18, 2024 •
edited
Loading

(I guess the right way to prevent something like this from happening again is probably asking folks to only leave REPLACEME in the PR, and let the git node land bump the string properly when it gets merged).

bnoordhuis added a commit to rubyjs/libv8-node that referenced this pull request Sep 19, 2024
@bnoordhuis
Update to node 22.9.0 (#62)
309d2ef 
Note the V8 version didn't change between 22.7.0 and 22.9.0 although
Node.js did add a new API in nodejs/node#54279

Remove unused and/or unnecessary patches. Upstream disabled Maglev so
we no longer have to do that manually. The genv8constants.py patches
were for a script that I removed upstream last year.
@targos targos added the dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. label Sep 22, 2024
louwers pushed a commit to louwers/node that referenced this pull request Nov 2, 2024
@joyeecheung @louwers
deps: V8: cherry-pick e74d0f437fcd
bdf0d66 
Original commit message:

    [api] add v8::Isolate::GetDefaultLocale()

    This allows embedders to query the default locale used by Intl
    APIs.

    This information is already available to user land via
    Intl?.Collator().resolvedOptions().locale, the issue with this
    is that it's a lot slower and requires dynamic access to Intl
    API, which is subject to patching and prototype pollution so it's
    not as reliable for embedders than having a V8 API to query
    the default locale directly.

    Refs: nodejs#54279
    Change-Id: I5a1823993c9ae79f8f61f54c6464daf882a09ba3
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5772938
    Reviewed-by: Leszek Swirski <leszeks@chromium.org>
    Commit-Queue: Joyee Cheung <joyee@igalia.com>
    Cr-Commit-Position: refs/heads/main@{#95678}

Refs: v8/v8@e74d0f4
PR-URL: nodejs#54279
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
louwers pushed a commit to louwers/node that referenced this pull request Nov 2, 2024
@joyeecheung @louwers
src: use v8::Isolate::GetDefaultLocale() to compute navigator.language
82d8cee 
Using the Intl API to get the default locale slows down the startup
significantly. This patch uses a new v8 API to get the default locale
directly.

PR-URL: nodejs#54279
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@ljharb ljharb ljharb approved these changes

@Ethan-Arrowood Ethan-Arrowood Ethan-Arrowood approved these changes

@legendecas legendecas legendecas approved these changes

@gengjiawen gengjiawen gengjiawen approved these changes

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@mcollina mcollina mcollina approved these changes

Assignees
No one assigned
Labels
commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. dependencies Pull requests that update a dependency file. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.