Skip to content

fs: add support for async iterators to fs.writeFile #5

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

Open
wants to merge 47 commits into
base: master
Choose a base branch
from

Conversation

yagipy
Copy link
Owner

@yagipy yagipy commented May 3, 2021

Fixes: nodejs#38075

@yagipy yagipy force-pushed the add-support-for-async-iterators-to-fs-writeFile branch 2 times, most recently from 9a563a1 to c9bba3f Compare May 4, 2021 08:33
Trott and others added 28 commits May 4, 2021 05:57
test-net-connect-econnrefused esems to run quickly on all platforms and
does not appear to need to be in the pummel directory. Move to
sequential.

PR-URL: nodejs#38462
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Use sentence case consistently in headers in src/crypto/README.md.

PR-URL: nodejs#38524
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Notable changes:

fs:
  * (SEMVER-MINOR) allow no-params fsPromises fileHandle read (Nitzan Uziely) nodejs#38287

PR-URL: nodejs#38522
PR-URL: nodejs#38500
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Adrian Estrada <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
When rendered by `man node`, all flags have a dash prepended to them.
Prior to this change, the man page would incorrectly read
`---unhandled-rejections=mode`

The three dashes is invalid syntax and rejected by the `node` command.

PR-URL: nodejs#38532
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
The name ‘gnu++1y’ is deprecated.

Refs: https://gcc.gnu.org/onlinedocs/gcc/C-Dialect-Options.html

PR-URL: nodejs#38504
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
PR-URL: nodejs#38516
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Mary Marchini <[email protected]>
PR-URL: nodejs#38502
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Both for consistency within the doc and to conform with IBM's usage
regarding their on product, refer to the operating system as AIX rather
than Aix.

PR-URL: nodejs#38535
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
The content we're linking to for AHAFS was published in 2011 but has now
been archived. This updates the link to the archived version (as the
current version results in a "Yikes!" page with an HTTP 400 response
code).

Unfortunately, the archived version does say "This content is no
longer being updated or maintained. The content is provided “as is.”
Given the rapid evolution of technology, some content, steps, or
illustrations may have changed." So we'll want to update at some point I
imagine. But for the moment, archived content with a notice like that is
better than a broken link.

PR-URL: nodejs#38534
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ash Cripps <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Added JSDoc typings for the `readline` lib module.

PR-URL: nodejs#38253
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Make it clearer in the docs that DiffieHellmanGroup does
not support changing the keys after creation.

PR-URL: nodejs#38363
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Adrian Estrada <[email protected]>
Invoke threadsafe_function during the same tick and avoid marshalling
costs between threads and/or churning event loop if either:

1. There's a queued call already
2. `Push()` is called while the main thread was running
   threadsafe_function

PR-URL: nodejs#38506
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Refs: https://coverage.nodejs.org/coverage-68e6673224365120/lib/internal/abort_controller.js.html#L126

PR-URL: nodejs#38514
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
PR-URL: nodejs#38520
Refs: https://coverage.nodejs.org/coverage-68e6673224365120/lib/querystring.js.html#L179
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
PR-URL: nodejs#38518
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Fixes: nodejs#38540
PR-URL: nodejs#38542
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
PR-URL: nodejs#38451
Fixes: nodejs#38373
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
This commit skips some test when OpenSSL 3.0.0-alpha15 is used as there
is an issue that causes them to fail.

This is only a temp solution until there is new OpenSSL release.

Fixes: nodejs#38373

PR-URL: nodejs#38451
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#38523
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#38530
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
This reverts commit 73370b4.

The unit test is preserved to make sure it does not break
nodejs#26463 again.

PR-URL: nodejs#38510
Fixes: nodejs#38499
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#38510
Fixes: nodejs#38499
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Wrap lines more than 80 chararcters long in inspect_repl.js so we can
disable specific rules.

PR-URL: nodejs#38529
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Avoid non-ASCII char in lib code as a single non-ASCII char forces all
the chars to be stored inefficiently, bloating the binary size. This
also brings the file closer to compatibility with our lint rules.

PR-URL: nodejs#38529
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#38529
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
In the user perspective, it's not an arguemnt type error. Replace it
with an `assert` expression.

PR-URL: nodejs#38519
Refs: https://coverage.nodejs.org/coverage-68e6673224365120/lib/internal/fs/promises.js.html#L268
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
pd4d10 and others added 15 commits May 6, 2021 10:39
Original commit message:

  Reland "[api] JSFunction PromiseHook for v8::Context"

  This is a reland of d5457f5fb7ea05ca05a697599ffa50d35c1ae3c7
  after a speculative revert.

  Additionally it fixes an issue with throwing promise hooks.

  Original change's description:
  > [api] JSFunction PromiseHook for v8::Context
  >
  > This will enable Node.js to get much better performance from async_hooks
  > as currently PromiseHook delegates to C++ for the hook function and then
  > Node.js delegates it right back to JavaScript, introducing several
  > unnecessary barrier hops in code that gets called very, very frequently
  > in modern, promise-heavy applications.
  >
  > This API mirrors the form of the original C++ function based PromiseHook
  > API, however it is intentionally separate to allow it to use JSFunctions
  > triggered within generated code to, as much as possible, avoid entering
  > runtime functions entirely.
  >
  > Because PromiseHook has internal use also, beyond just the Node.js use,
  > I have opted to leave the existing API intact and keep this separate to
  > avoid conflicting with any possible behaviour expectations of other API
  > users.
  >
  > The design ideas for this new API stemmed from discussion with some V8
  > team members at a previous Node.js Diagnostics Summit hosted by Google
  > in Munich, and the relevant documentation of the discussion can be found
  > here: https://docs.google.com/document/d/1g8OrG5lMIUhRn1zbkutgY83MiTSMx-0NHDs8Bf-nXxM/edit#heading=h.w1bavzz80l1e
  >
  > A summary of the reasons for why this new design is important can be
  > found here: https://docs.google.com/document/d/1vtgoT4_kjgOr-Bl605HR2T6_SC-C8uWzYaOPDK5pmRo/edit?usp=sharing
  >
  > Bug: v8:11025
  > Change-Id: I0b403b00c37d3020b5af07b654b860659d3a7697
  > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2759188
  > Reviewed-by: Marja Hölttä <[email protected]>
  > Reviewed-by: Camillo Bruni <[email protected]>
  > Reviewed-by: Anton Bikineev <[email protected]>
  > Reviewed-by: Igor Sheludko <[email protected]>
  > Commit-Queue: Camillo Bruni <[email protected]>
  > Cr-Commit-Position: refs/heads/master@{#73858}

  Bug: v8:11025
  Bug: chromium:1197475
  Change-Id: I73a71e97d9c3dff89a2b092c3fe4adff81ede8ef
  Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2823917
  Reviewed-by: Marja Hölttä <[email protected]>
  Reviewed-by: Igor Sheludko <[email protected]>
  Reviewed-by: Anton Bikineev <[email protected]>
  Reviewed-by: Camillo Bruni <[email protected]>
  Commit-Queue: Camillo Bruni <[email protected]>
  Cr-Commit-Position: refs/heads/master@{#74071}

Refs: v8/v8@c0fceaa

PR-URL: nodejs#36394
Reviewed-By: Bryan English <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Original commit message:

    [runtime] Fix promise hooks

    promiseCapability can be undefined.

    Bug: v8:11025
    Bug: chromium:1201113
    Change-Id: I9da8764820cee0db1f0c38ed2fff0e3afeb9a80e
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2844649
    Reviewed-by: Marja Hölttä <[email protected]>
    Commit-Queue: Camillo Bruni <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#74117}

Refs: v8/v8@272445f

PR-URL: nodejs#36394
Reviewed-By: Bryan English <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Original commit message:

    [promises] Change context promise hooks to Callable

    The previously added perf-context Promise-hooks take a v8::Function as
    arguments. However, the builtin code was only accepting JSFunctions
    which causes cast errors.

    Drive-by-fix: Directly pass nativeContext in more places.

    Bug: chromium:1201465
    Change-Id: Ic8bed11253a1f18a84e71eb9ea809b1ec1c3f428
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2850162
    Reviewed-by: Jakob Gruber <[email protected]>
    Commit-Queue: Camillo Bruni <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#74223}

Refs: v8/v8@5f44131

PR-URL: nodejs#36394
Reviewed-By: Bryan English <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Original commit message:

    [promises] Fix slow path when context promise hooks are present

    Bug: chromium:1201936
    Change-Id: I1ee545e33587ddf4a5c7e1cbd64b53d36c75a146
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2850936
    Reviewed-by: Marja Hölttä <[email protected]>
    Reviewed-by: Jakob Gruber <[email protected]>
    Commit-Queue: Camillo Bruni <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#74267}

Refs: v8/v8@4c07451

PR-URL: nodejs#36394
Reviewed-By: Bryan English <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Original commit message:

    [runtime] Fix Promise.all context promise hooks

    We have to take the slow path in Promise.all if context promise hooks
    are set. The fast-path doesn't create intermediate promises by default.

    Bug: chromium:1204132, v8:11025
    Change-Id: Ide92de00a4f6df05e0ddbc8814f6673bd667f426
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2866771
    Reviewed-by: Victor Gomes <[email protected]>
    Commit-Queue: Camillo Bruni <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#74326}

Refs: v8/v8@fa4cb17

PR-URL: nodejs#36394
Reviewed-By: Bryan English <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#36394
Reviewed-By: Bryan English <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: James M Snell <[email protected]>
This avoids loading the entirety of `node:util` and `node:url` and their
dependencies while only a subset is actually used by this module.

PR-URL: nodejs#38550
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
PR-URL: nodejs#38541
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
1. custom inspect test case
Refs: https://coverage.nodejs.org/coverage-18e4f405b14b26f9/lib/internal/histogram.js.html#L56

2. tests that RecordableHistogram is impossible to construct manually
Refs: https://coverage.nodejs.org/coverage-18e4f405b14b26f9/lib/internal/histogram.js.html#L132

PR-URL: nodejs#38555
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Refs: v8/v8@9.0.257.24...9.0.257.25

PR-URL: nodejs#38556
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#38537
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
The `torque_generated_definitions` target is missing some torque
generated files in its sources list when compared to the equivalent
target in V8's BUILD.gn.

PR-URL: nodejs#38576
Fixes: nodejs#38571
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
All (or at least most) of the tests uses lambdas (or arrow functions
if you will) to call these functions internally inside of directly
calling them, this should also use this technique for consistency.

PR-URL: nodejs#38560
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
PR-URL: nodejs#38566
Reviewed-By: Ujjwal Sharma <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@yagipy yagipy force-pushed the add-support-for-async-iterators-to-fs-writeFile branch 5 times, most recently from a7cc927 to 3313df8 Compare May 8, 2021 13:51
@yagipy yagipy force-pushed the add-support-for-async-iterators-to-fs-writeFile branch 3 times, most recently from ba733d5 to ccdb3d1 Compare May 9, 2021 02:17
@yagipy yagipy force-pushed the add-support-for-async-iterators-to-fs-writeFile branch from ccdb3d1 to 6f21783 Compare May 9, 2021 02:50
@yagipy yagipy force-pushed the add-support-for-async-iterators-to-fs-writeFile branch from 176573e to c0b87d6 Compare May 15, 2021 05:17
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.

fs: add support for async iterators to fs.writeFile