[v14.x] src: use correct outer Context’s microtask queue by addaleax · Pull Request #38787 · nodejs/node · GitHub

Backports #36482 and #36581 to v14.x. Only conflicts were neighbouring-line conflicts in the V8 cctest file and common.gypi, so I don’t think this needs actual review, just CI.

I added a new custom ESLint rule to fix these problems. We have a lot of replaceable codes with primordials. Accessing built-in objects is restricted by existing rule (no-restricted-globals), but accessing property in the built-in objects is not restricted right now. We manually review codes that can be replaced by primordials, but there's a lot of code that actually needs to be fixed. We have often made pull requests to replace the primordials with. Restrict accessing global built-in objects such as `Promise`. Restrict calling static methods such as `Array.from` or `Symbol.for`. Don't restrict prototype methods to prevent false-positive. PR-URL: nodejs#35448 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Ben Coe <bencoe@gmail.com>

PR-URL: nodejs#36017 Reviewed-By: Shingo Inoue <leko.noor@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

Make the linter recommend replacing `globalThis.Map` by `primordials.SafeMap`, and similar for `Set`, `WeakSet`, and `WeakMap`. PR-URL: nodejs#36026 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Shingo Inoue <leko.noor@gmail.com>

Co-authored-by: Benjamin Gruenbaum <benjamingr@gmail.com> Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: nodejs#36006 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Refs: nodejs#35448 Refs: nodejs#36003 Refs: https://tc39.es/ecma262/#sec-%typedarray%-intrinsic-object Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com> PR-URL: nodejs#36016 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Shingo Inoue <leko.noor@gmail.com>

PR-URL: nodejs#36024 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Update ESLint to 7.11.0 PR-URL: nodejs#35578 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

Update ESLint to 7.12.0 PR-URL: nodejs#35799 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>

Update ESLint to 7.12.1 PR-URL: nodejs#35799 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>

Update ESLint to 7.13.0 PR-URL: nodejs#36031 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

This makes sure that all async functions finish as expected. PR-URL: nodejs#34363 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

The result of an async IIFE should always be handled in our tests, typically by adding `.then(common.mustCall())` to verify that the async function actually finishes executing at some point. PR-URL: nodejs#34363 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

It's possible to annotate failures in Actions by printing "::error file={},line={},col={}::{message}". This methos is preferrable over using a problem matcher because problem matchers only allow single-line messages, whereas ::error allows multi-line messages. PR-URL: nodejs#34590 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com>

Fixes: nodejs#35237 PR-URL: nodejs#35257 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

Fixes: nodejs#35740 Refs: nodejs#31553 Refs: nodejs#32953 Refs: nodejs#33991 Refs: nodejs#34001 Refs: nodejs#34055 Refs: nodejs#34962 Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com> Co-authored-by: Trivikram Kamat <trivikr.dev@gmail.com> Co-authored-by: Rich Trott <rtrott@gmail.com> PR-URL: nodejs#34002 Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Masashi Hirano <shisama07@gmail.com> Reviewed-By: Pranshu Srivastava <rexagod@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

PR-URL: nodejs#36003 Reviewed-By: Rich Trott <rtrott@gmail.com>

PR-URL: nodejs#36025 Reviewed-By: Shingo Inoue <leko.noor@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

PR-URL: nodejs#36132 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

PR-URL: nodejs#36141 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

PR-URL: nodejs#36140 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: nodejs#36167 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

In preparation for a cpplint version bump, move labels to be unindented. The one space indentation we use now is flagged in cpplint 1.5.2. Refs: https://softwareengineering.stackexchange.com/q/151806/26510 PR-URL: nodejs#36213 Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>

Update ESLint to 7.14.0 PR-URL: nodejs#36217 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

This rule is new in ESLint 7.14.0. PR-URL: nodejs#36217 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

This is done to avoid creating an array and gain performance. Co-authored-by: ExE Boss <3889017+ExE-Boss@users.noreply.github.com> PR-URL: nodejs#36221 Refs: https://v8.dev/blog/v8-release-80#optimizing-higher-order-builtins Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>

Adds methods for fetching stack trace information about enclosing function. Refs nodejs#36042 Original commit message: Reland "stack-trace-api: implement getEnclosingLine/Column" This reverts commit 5557a63beb5a53c93e9b590eaf2933e21bcb3768. Reason for revert: Sheriff's mistake, failing test was previously flaking. Original change's description: > Revert "stack-trace-api: implement getEnclosingLine/Column" > > This reverts commit c48ae2d96cbfdc2216706a5e9a79ae1dce5a638b. > > Reason for revert: Breaks a profiling test: > https://ci.chromium.org/p/v8/builders/ci/V8%20Win32/30010 > > Original change's description: > > stack-trace-api: implement getEnclosingLine/Column > > > > Introduces getEnclosingColumn and getEnclosingLine on CallSite > > so that the position can be used to lookup the original symbol > > for function when source maps are used. > > > > BUG=v8:11157 > > > > Change-Id: I06c4c374d172d206579abb170c7b7a2bd3bb159f > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2547218 > > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> > > Commit-Queue: Benjamin Coe <bencoe@google.com> > > Cr-Commit-Position: refs/heads/master@{#71343} > > TBR=jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com > > Change-Id: Iab5c250c1c4fbdab86971f4a7e40abc8f87cf79c > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: v8:11157 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555384 > Reviewed-by: Bill Budge <bbudge@chromium.org> > Commit-Queue: Bill Budge <bbudge@chromium.org> > Cr-Commit-Position: refs/heads/master@{#71345} TBR=bbudge@chromium.org,jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com # Not skipping CQ checks because this is a reland. Bug: v8:11157 Change-Id: I8dba19ceb29a24594469d2cf79626f741dc4cad3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555499 Reviewed-by: Bill Budge <bbudge@chromium.org> Commit-Queue: Bill Budge <bbudge@chromium.org> Cr-Commit-Position: refs/heads/master@{#71348} Refs: v8/v8@86991d0 PR-URL: nodejs#36254 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

PR-URL: nodejs#36210 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Bradley Farias <bradley.meck@gmail.com>

PR-URL: nodejs#36272 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

PR-URL: nodejs#36284 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

PR-URL: nodejs#36285 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Original commit message: [api] Add Context::GetMicrotaskQueue method Add a method that returns the microtask queue that is being used by the `v8::Context`. This is helpful in non-monolithic embedders like Node.js, which accept Contexts created by its own embedders like Electron, or for native Node.js addons. In particular, it enables: 1. Making sure that “nested” `Context`s use the correct microtask queue, i.e. the one from the outer Context. 2. Enqueueing microtasks into the correct microtask queue. Previously, these things only worked when the microtask queue for a given Context was the Isolate’s default queue. As an alternative, I considered adding a way to make new `Context`s inherit the queue from the `Context` that was entered at the time of their creation, but that seemed a bit more “magic”, less flexible, and didn’t take care of concern 2 listed above. Change-Id: I15ed796df90f23c97a545a8e1b30a3bf4a5c4320 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2579914 Reviewed-by: Toon Verwaest <verwaest@chromium.org> Commit-Queue: Toon Verwaest <verwaest@chromium.org> Cr-Commit-Position: refs/heads/master@{#71710} Refs: v8/v8@4bf051d PR-URL: nodejs#36482 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

Fall back to using the outer context’s microtask queue, rather than the Isolate’s default one. This would otherwise result in surprising behavior if an embedder specified a custom microtask queue for the main Node.js context. PR-URL: nodejs#36482 Refs: v8/v8@4bf051d Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

I missed in c6c8337 that we should not just use that queue for enqueuing microtasks, but also for running them. Refs: nodejs#36482 PR-URL: nodejs#36581 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gus Caplan <me@gus.host>

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

Looks like the change depends on another PR that's not in v14.x yet:

17:00:40 ../test/cctest/test_environment.cc: In member function ‘virtual void EnvironmentTest_NestedMicrotaskQueue_Test::TestBody()’:
17:00:40 ../test/cctest/test_environment.cc:718:19: error: ‘SpinEventLoop’ was not declared in this scope
17:00:40    int exit_code = SpinEventLoop(env).FromJust();
17:00:40                    ^~~~~~~~~~~~~

Edit: it's #35597

@addaleax do you still want this to land on v14.x ? A rebase is needed and there's an issue with a test that is using an API that doesn't exist in v14.x.

If this is still desired for v14.x it needs a rebase and #38787 (comment) addressed.

Closing as v14.x is EoL.

targos and others added 30 commits May 16, 2021 15:10

tools,lib: tighten prefer-primordials rules for Error statics

5068db6

PR-URL: nodejs#36017 Reviewed-By: Shingo Inoue <leko.noor@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

module: refactor to use more primordials

1f38c63

PR-URL: nodejs#36024 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

tools: update ESLint to 7.11.0

64c5f62

Update ESLint to 7.11.0 PR-URL: nodejs#35578 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

tools: update ESLint to 7.12.0

8b2df0c

Update ESLint to 7.12.0 PR-URL: nodejs#35799 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>

tools: update ESLint to 7.12.1

8e4a055

Update ESLint to 7.12.1 PR-URL: nodejs#35799 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>

tools: update ESLint to 7.13.0

17c8484

Update ESLint to 7.13.0 PR-URL: nodejs#36031 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

stream: fix legacy pipe error handling

22bf488

Fixes: nodejs#35237 PR-URL: nodejs#35257 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

child_process: refactor to use more primordials

b566802

PR-URL: nodejs#36003 Reviewed-By: Rich Trott <rtrott@gmail.com>

lib,tools: enforce access to prototype from primordials

483c1a1

PR-URL: nodejs#36025 Reviewed-By: Shingo Inoue <leko.noor@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

timers: refactor to use more primordials

8c743ca

PR-URL: nodejs#36132 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

http2: add support for TypedArray to getUnpackedSettings

4d35416

PR-URL: nodejs#36141 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

lib: refactor to use more primordials

986e79a

PR-URL: nodejs#36140 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

errors: refactor to use more primordials

34984f3

PR-URL: nodejs#36167 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

tty: refactor to use more primordials

b641c26

PR-URL: nodejs#36272 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

os: refactor to use more primordials

1462d59

PR-URL: nodejs#36284 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

v8: refactor to use more primordials

d49ec9b

PR-URL: nodejs#36285 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

addaleax added 3 commits May 23, 2021 16:43

src: use correct microtask queue for checkpoints

df3d763

I missed in c6c8337 that we should not just use that queue for enqueuing microtasks, but also for running them. Refs: nodejs#36482 PR-URL: nodejs#36581 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gus Caplan <me@gus.host>

github-actions 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. v14.x labels May 23, 2021

jasnell approved these changes May 23, 2021

View reviewed changes

targos force-pushed the v14.x-staging branch 4 times, most recently from 49c8441 to 50b3bd0 Compare June 11, 2021 07:31

targos force-pushed the v14.x-staging branch from 50b3bd0 to f04a702 Compare June 15, 2021 12:40

richardlau force-pushed the v14.x-staging branch 2 times, most recently from 1f06fcf to 16dcd9c Compare July 5, 2021 16:02

richardlau force-pushed the v14.x-staging branch from 8ea39c6 to 4ac719f Compare July 19, 2021 11:19

richardlau force-pushed the v14.x-staging branch from 073941c to 0061e5d Compare July 29, 2021 16:40

BethGriggs force-pushed the v14.x-staging branch from 78bb65e to 73e6781 Compare August 12, 2021 16:49

MylesBorins force-pushed the v14.x-staging branch from b3f51ee to 327838d Compare August 31, 2021 14:58

danielleadams force-pushed the v14.x-staging branch from 8216d46 to ec3bd72 Compare October 12, 2021 14:30

richardlau force-pushed the v14.x-staging branch from b5b66ef to 1f48dd8 Compare November 30, 2021 16:12

richardlau force-pushed the v14.x-staging branch from 495dde6 to cede1f2 Compare January 10, 2022 23:12

richardlau force-pushed the v14.x-staging branch 2 times, most recently from 30ba86c to 7f2825b Compare March 17, 2022 23:36

targos closed this May 3, 2023

jshaofa-ui mentioned this pull request May 22, 2026

[BUG] Edit tool silently no-ops on disk while Read returns the edited content (sub-agent worktrees + main session) anthropics/claude-code#61471

Closed

3 tasks

[v14.x] src: use correct outer Context’s microtask queue by addaleax · Pull Request #38787 · nodejs/node · GitHub

Uh oh!

Uh oh!

[v14.x] src: use correct outer Context’s microtask queue - #38787

Hidden character warning

[v14.x] src: use correct outer Context’s microtask queue#38787
addaleax wants to merge 66 commits into
nodejs:v14.x-stagingfrom
addaleax:backport-v14.x-µtaskqueue

addaleax commented May 23, 2021

Uh oh!

nodejs-github-bot commented May 23, 2021

Uh oh!

targos commented May 25, 2021 •
edited
Loading

Uh oh!

Uh oh!

targos commented Aug 8, 2021

Uh oh!

richardlau commented Jan 25, 2022

Uh oh!

targos commented May 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

18 participants

[v14.x] src: use correct outer Context’s microtask queue by addaleax · Pull Request #38787 · nodejs/node · GitHub

Uh oh!

Uh oh!

[v14.x] src: use correct outer Context’s microtask queue - #38787

Hidden character warning

[v14.x] src: use correct outer Context’s microtask queue#38787addaleax wants to merge 66 commits intonodejs:v14.x-stagingnodejs/node:v14.x-stagingfrom addaleax:backport-v14.x-µtaskqueueaddaleax/node:backport-v14.x-µtaskqueueCopy head branch name to clipboard

Conversation

addaleax commented May 23, 2021

Uh oh!

nodejs-github-bot commented May 23, 2021

Uh oh!

targos commented May 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

targos commented Aug 8, 2021

Uh oh!

richardlau commented Jan 25, 2022

Uh oh!

targos commented May 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

18 participants

[v14.x] src: use correct outer Context’s microtask queue#38787
addaleax wants to merge 66 commits into
nodejs:v14.x-stagingfrom
addaleax:backport-v14.x-µtaskqueue

targos commented May 25, 2021 •
edited
Loading