Skip to content

node:fs/promises: declare Node's parameters so each function has Node's length - #48555

Open
robobun wants to merge 2 commits into
mainfrom
robobun/c7b32ff3/fs-promises-function-length
Open

robobun wants to merge 2 commits into
mainfrom
robobun/c7b32ff3/fs-promises-function-length

Conversation

@robobun

@robobun robobun commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • require("node:fs/promises").mkdir.length is 3. Node v26.12.1 gives 2. 23 of the 32 shared functions differ (24 on macOS).
  • asyncWrap copies the native operation's length (src/js/node/fs.promises.ts:462): the callback form's arity, or 0 under --trace-event-categories node.fs.
  • _.curry(fsPromises.mkdir)(dir, options) returns a function and makes no directory.

Fix

  • 16 functions (17 on macOS) and readFile declare Node's parameters and call the native operation by name, like node:os (One bindgen: replace bindgen v1 and v2, add classes, generate the Rust side #45104). asyncWrap takes the length as an argument. Four more state theirs.
  • Correct because each function settles as its sync form does.
  • Verified: test/js/node/fs/promises.test.js (32 new tests, 5 fail on main), test/js/node/fs, 370 upstream files, AOT checks.
  • Self-reviewed: 23 concerns raised, 11 addressed, 11 in part. Not taken: declaring chown and five more now changes ten error texts.

Background

  • length counts the parameters before the first default or rest parameter. = undefined stands where Node has a default.
  • Some native parsers tell a missing argument from undefined (uid is required) until bindgen: delete b.any and .variadic #46789. Their exports pass on the caller's exact arguments.
  • Considered a stated length at all 31 asyncWrap sites: no call path changes, but each number stands beside ...args.

Downsides

  • Five lengths go up (appendFile, open, opendir, readFile, writeFile). test("x", fsPromises.readFile.bind(null, file)) passes under bun test today, then waits for done, as under Jest.
  • Each declared function is its own code block: about 43,000 more instructions at its first call, and 1,179,239 more for five in a 100-iteration loop. The binary grows 1,046 bytes.
  • Seven Bun-only lengths still count a callback. Nine of node:fs still differ (fs.stat).
Notes

Origin. A differential run of node:fs/promises against Node found this. No user reported it, so there is no issue (#44716). #45104 fixed the length of seven node:os functions the same way and pins them with a table (test/js/node/os/os.test.js:323).

Script.

const fsPromises = require("node:fs/promises");
const out = {};
for (const k of Object.keys(fsPromises).sort()) if (typeof fsPromises[k] === "function") out[k] = fsPromises[k].length;
console.log(JSON.stringify(out));

The 23 lengths (Linux x64, main 89452d2). Node v26.3.0 and v26.12.1 print the same numbers.

function main Node v26.12.1, this PR Node's declaration how this PR states it
access 3 1 access(path, mode = F_OK) declared
appendFile 1 3 appendFile(path, data, options) stated after the table
chmod 3 2 chmod(path, mode) declared
chown 4 3 chown(path, uid, gid) asyncWrap argument
copyFile 4 3 copyFile(src, dest, mode) declared
lchown 4 3 lchown(path, uid, gid) asyncWrap argument
link 3 2 link(existingPath, newPath) declared
lutimes 4 3 lutimes(path, atime, mtime) asyncWrap argument
mkdir 3 2 mkdir(path, options) declared
mkdtemp 3 2 mkdtemp(prefix, options) declared
open 1 3 open(path, flags, mode) stated after the table
opendir 2 3 promisify(opendir), which copies the 3 of opendir(path, options, callback) stated after the table
readFile 1 2 readFile(path, options) declared
readdir 3 2 readdir(path, options) declared
readlink 3 2 readlink(path, options) declared
realpath 3 2 realpath(path, options) declared
rename 3 2 rename(oldPath, newPath) declared
statfs 2 1 statfs(path, options = { ... }) declared
symlink 4 3 symlink(target, path, type) declared
truncate 3 1 truncate(path, len = 0) declared
unlink 2 1 unlink(path) declared
utimes 4 3 utimes(path, atime, mtime) asyncWrap argument
writeFile 1 3 writeFile(path, data, options) stated after the table
  • stat and lstat were 1 and stay 1. They are declared now, so the number no longer comes from the native table.
  • On macOS lchmod went through asyncWrap and took 3 where Node has 2. It is declared now. I read that arm and did not run it.
  • FileHandle.prototype.writeFile goes from 1 to 2: its options parameter had a default that the body did not need. The other 18 shared methods were equal.
  • Node's source: lib/internal/fs/promises.js#L1418-L2366 and lib/internal/fs/dir.js#L330.

Lengths in six ways to load the module. Shared names that differ from Node v26.12.1, of 32:

require import * fs.promises --trace-event-categories node.fs tracing enabled before the first require a Worker
main 23 23 23 25 25 23
this PR 0 0 0 0 0 0

With a node.fs trace category on, main wraps each native operation in a function of length 0, and asyncWrap copies that. There stat and lstat read 0 too.

Why declared parameters. Node states 31 of its 32 numbers as parameter lists. rm, rmdir, cp and mkdtempDisposable in this file already are declared functions that call the native operation by name, and #45104 gave node:os this form. A declared list cannot drift from its number. It does not depend on the native table or on the trace wrappers, and it needs no Object.defineProperty call. The cost of a call is not the reason: see Measurements.

Why six functions pass on the caller's exact arguments. The native parser of each tells a missing argument from undefined, and Node does not:

call main and this PR the same with undefined in that place
chown(path), lchown(path) uid is required The "uid" argument must be of type number. Received undefined
chown(path, 0), lchown(path, 0) gid is required The "gid" argument must be of type number. Received undefined
utimes(path), lutimes(path) atime is required atime must be a number or a Date
utimes(path, 1), lutimes(path, 1) mtime is required mtime must be a number or a Date
writeFile(path), appendFile(path) data is required The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView

A declared parameter list would turn the first column into the second. This PR changes no error text. #46789 (open) removes the first column from the native parsers. After it these six can take Node's parameter lists too, and asyncWrap is left to the ten Bun-only functions. No test here names a text of either column, so this PR and #46789 can merge in either order.

The functions that only Bun exports. close 1, exists 0, fchmod 3, fchown 4, fdatasync 2, fstat 1, fsync 2, ftruncate 1, futimes 4, read 6, readv 3, write 6, writev 3. They keep their numbers. Seven of them (fchmod, fchown, fdatasync, fsync, futimes, read, write) still count the callback of the callback form, so fchown reads 4 beside chown 3. Node has no function of these names in fs/promises to take a number from, and this change is about the 32 shared names. On main the ten that go through asyncWrap read 0 under a node.fs trace category. Here they keep their numbers there.

Tests. 32 new tests in test/js/node/fs/promises.test.js (33 on macOS, with lchmod).

  • 5 fail on main: the table of Node's 32 lengths, the FileHandle method lengths, the table in a child with --trace-event-categories node.fs, the table in a child that replaced Object.defineProperty before the first require, and the trace events with tracing enabled after the first require.
  • 27 pass on main too and guard the new bodies. One runs the table under Node where Node v26.3.0 or later is installed. One pins the 13 Bun-only lengths. One checks names, the AsyncFunction kind and attributes. One checks the trace events with the command-line flag. 23 check one function each against its sync form.
  • The 23: each of the 16 declared functions, readFile, and the six that get the exact arguments is called with each prefix of a list, each prefix followed by undefined, and the whole list with two more arguments. The last argument of a list is one whose effect shows ({ bigint: true }, { encoding: "hex" }, COPYFILE_EXCL onto an existing file, an access mode of 8). The promise form must settle as the sync form returns or throws for the same arguments, and leave the same entries on disk. The sync form gets the caller's list as it is. 175 lists agree on main and on this PR.
  • 96 one-line mutations of the built module that the debug build loads (another operation, a dropped argument, two swapped arguments, a parameter or a default more or less, another stated length, the public Object.defineProperty, a copied fn.length): 80 fail a new test. The other 16 change nothing that a caller can see: 15 pass one more argument to a parser that reads none past Node's list, and one calls the other realpath operation of the binding, which answers the same for these paths.

Measurements. Release builds of main (89452d2) and of this PR on it, Linux x64. Instructions are user-mode instructions of the main thread (perf_event_open), with BUN_JSC_useConcurrentJIT=0 and the collector off. A second series of main is beside each result.

  • Size (size): text 143,094,946 to 143,095,992 (+1,046 bytes). data and bss are equal. The bundled module grows from 43,824 to 44,870 bytes. The generated native binding is unchanged.
  • First require("node:fs/promises"), median of 12 interleaved runs: 12,198,880 to 12,199,262 (+382). Main again: -2,192.
  • The first call. On main 31 exports share one function body. Here each declared function has its own, and makes its own bytecode at its first call. 17 awaited calls that work, of which n are of different declared functions (median of 9 runs):
n main this PR difference main again
1 19,544,639 19,544,776 +137 +2,627
5 19,669,421 19,844,495 +175,074 -6,121
9 19,768,854 20,110,365 +341,511 +11,025
13 19,881,806 20,409,288 +527,482 +18,311
17 20,354,532 21,187,744 +833,212 +56,962
  • Five declared functions in one loop (stat, readdir, mkdir, access, readFile, one awaited call of each for each iteration, median of 7 runs). Each function is compiled on its own, here on the main thread. So the total is higher until about 1,000 iterations and lower after that, where main's one shared body serves five different operations:
iterations main this PR difference main again
1 19,541,059 19,680,802 +139,743 -3,044
10 20,090,315 20,173,468 +83,153 +177,114
100 26,141,343 27,320,582 +1,179,239 -86,852
300 33,449,669 33,996,552 +546,883 -39,861
1,000 56,851,413 56,992,795 +141,382 +263,335
3,000 139,945,805 132,439,183 -7,506,622 -647,647
10,000 344,355,382 335,359,747 -8,995,635 -1,535,439
  • One awaited call that works, one export in a process (the slope between 20,000 and 60,000 calls, the least of 3 runs). Main again differs from main by -163 to +78:
export interpreter (BUN_JSC_useJIT=0) Baseline (BUN_JSC_useDFGJIT=0) default flags
stat 8,163 to 7,420 (-743) 6,655 to 5,971 (-684) 5,758 to 5,746 (-12)
mkdir 8,716 to 7,872 (-844) 7,045 to 6,339 (-706) 6,007 to 5,964 (-43)
readFile 10,024 to 8,501 (-1,523) 7,776 to 6,651 (-1,125) 6,285 to 6,294 (+9)
open and close 37,732 to 37,666 (-66) 18,761 to 18,785 (+24) 13,845 to 13,802 (-43)
  • One awaited call that the native parser rejects at once (a Symbol as the path). No thread-pool work is in this count, so main again differs by 3 at most. stat, readdir, mkdir, access, copyFile: -627 to -792 in the interpreter, -526 to -607 in Baseline, -390 to -486 with default flags. readFile: -1,933 to -2,189, -1,699 to -1,930, -1,421 to -1,544. chown, fstat, open, opendir, writeFile: -8 to +4.
  • 16 declared functions, 20,000 rejected calls each: 3,306,871,114 to 3,152,445,837 instructions (-4.7%). FunctionCodeBlock cells afterwards, collector on: 18 to 49. After one call of each: 12 to 27.
  • Heap cells that one call leaves with the collector and the JIT off (heapStats().objectTypeCounts): stat and mkdir lose 1 Array. readFile loses 2 Array cells and 1 butterfly. chown, fstat, writeFile and open are equal.
  • Calls of the public Object.defineProperty on a function during the first require: with key length 30 to 0, with key name 47 to 17.

Details of the text that a measurement decided. The first three are from builds of an earlier form of this change on main 2596ac5.

  • The descriptors are { value }, as on main. A { __proto__: null, value } literal cost about 1,700 instructions each at load (+50,000 for the module), and one shared null-prototype descriptor cost +67,000.
  • open keeps its text and gets a stated length. With (path, flags, mode) and two === undefined checks, one rejected call cost +58 instructions in the interpreter and +249 with default flags.
  • The declared functions are anonymous function expressions, like appendFile and writeFile. Named ones cost +19,000 instructions at load and 12 to 19 more per call.
  • The FileHandle constructor keeps its length of 3 (Node: 1). Two default parameters there cost open +110 instructions per call in the interpreter.

Other shapes that I measured.

  • A stated length at all 31 asyncWrap sites. No call path changes and no first call costs more. The numbers then stand beside ...args, 30 definitions stay at load, and tracing that starts late sees none of them. If no measured case may move, this is the shape to take.
  • One shared wrapper with three fixed parameters and a stated length. No first-call cost for each export, but +146,000 instructions at the first require.

What changes besides the numbers.

  • A bound function as the body of a test under bun test. Five kinds of call that pass on main fail here, and three that fail on main pass here. The next paragraph has the table.
  • Tracing enabled after the first require (trace_events.createTracing().enable(), the usual order in an ES module, where the import runs first): a declared function reads the native operation at each call, so it is traced. Of 25 exports called once, main traces 2 and this PR 18. Node v26.12.1 traces all 25. Still not traced in that order: chown, lchown, utimes, lutimes, writeFile, appendFile, the ten Bun-only functions and the FileHandle methods that call them. statfs is traced in no order. With --trace-event-categories the events are the same as on main.
  • A script that replaced Object.defineProperty before the first require with a function that defines nothing on a function: on main 25 of the 32 shared names then read a wrong name (wrapped) or length. Here none does.
  • Function.prototype.toString of a declared function prints its own text.

A bound function as the body of a test. bun test passes done to a test callback with a length of 1 or more and waits for it (src/runtime/test_runner/ScopeFunctions.rs:463). Jest does the same. So test("x", fsPromises.readFile.bind(null, file)) passes on main, where the bound function has a length of 0. Here the length is 1 and the test fails: this test timed out after 5000ms, before its done callback was called. If a done callback was not intended, remove the last parameter from the test callback function. Jest 30.5.2 on Node v26.12.1 fails the same test: Exceeded timeout of 5000 ms for a test while waiting for `done()` to be called. 11 bound calls as test bodies, each with the length of the bound function:

test body Jest on Node main this PR
fsPromises.readFile.bind(null, file) fails, 1 passes, 0 fails, 1
fsPromises.writeFile.bind(null, file, "data") fails, 1 passes, 0 fails, 1
fsPromises.appendFile.bind(null, file, "data") fails, 1 passes, 0 fails, 1
fsPromises.open.bind(null, file) fails, 2 passes, 0 fails, 2
fsPromises.open.bind(null, file, "r", 0o666) passes, 0 passes, 0 passes, 0
fsPromises.opendir.bind(null, dir) fails, 2 fails, 1 fails, 2
fsPromises.opendir.bind(null, dir, {}) fails, 1 passes, 0 fails, 1
fsPromises.mkdir.bind(null, dir, { recursive: true }) passes, 0 fails, 1 passes, 0
fsPromises.unlink.bind(null, file) passes, 0 fails, 1 passes, 0
fsPromises.access.bind(null, file, 0) passes, 0 fails, 1 passes, 0
fs.readFile.bind(null, file) passes, 2 passes, 2 passes, 2

Main differs from Jest in 8 of the 11 outcomes, and this PR in none. The rule of the test runner stays as it is: Jest has it, and the last row needs it.

The same class outside this PR.

  • node:fs: 9 of 100 shared function exports differ from Node in length (Dir 3/4, Stats 18/0, close 1/2, fstat 1/3, lstat 1/3, openAsBlob 1/2, stat 1/3, statfs 1/3, statfsSync 1/2, Node/Bun), and fs.realpathSync.native 2/3. So stat reads 1 in its promise form and 3 in its callback form. Seven of them are one-line edits of the same kind. They are not in this PR: node:fs has a second text for --compile --aot since AOT: the next base #48120, and a default parameter on fs.stat and fs.close needs numbers of its own.
  • The FileHandle constructor: 1/3. See above.
  • 32 node:fs exports (31 sync functions and opendir) read 0 under --trace-event-categories node.fs, on main and here. The wrappers in src/js/internal/trace_events.ts do not copy length.
  • All node:* modules: 173 of 853 top-level function exports that both runtimes have differ in length on main, and 150 with this PR (Node v26.12.1, modules that load without a flag, an alias module counted again). process: give emitWarning and 15 more functions the length Node gives them #48051 changes process.

Open PRs on the same lines.

Self-review. 23 concerns. 11 are addressed in full, 11 in part, and 1 is not taken.

  • In full: the test of the exact arguments is the sync-form test and names no error text. The test of the declared functions no longer compares a function with itself. The table runs under Node where Node is installed. The rows of the Bun-only functions are out of the table of Node's lengths. This text gives the first-call cost, the compile cost and the mixed loop, and does not give the cost of a call as the reason for the shape.
  • In part: FileHandle.prototype.writeFile is in, and node:fs and the constructor are named above with the reasons. The Bun-only lengths stay and are named. The trace wrappers are named, not changed. The list of open PRs names the ones that I read.
  • Not taken: to declare chown, lchown, utimes, lutimes, writeFile and appendFile now. That changes ten error texts before bindgen: delete b.any and .variadic #46789 does.
  • Not done: a tracking issue for the other modules. A maintainer closed such issues as not for this tracker (undici.request(): the method is upper-cased, and a method that is not a string becomes GET #44716).

Search for programs that read these lengths. A GitHub code search for promises.<name>.length ran for 6 of the 23 names (writeFile, readFile, open, opendir, access, readdir) before the search quota ended. It found no program that branches on one of the numbers. fs2 copies fs.promises.open.length onto its own wrapper.

What ran.

  • bun bd test test/js/node/fs/promises.test.js on the debug ASAN build: 62 pass, 5 skip (all five were skipped before), 0 fail. On main's release build: 57 pass, 5 fail.
  • 34 test files on release builds of main and of this PR (test/js/node/fs, test/js/node/trace_events, the two visible-shape files, the readline history test, worker_threads.test.ts): all pass on this PR. On main the 5 tests above fail, and one test of fs.test.ts (keeps a non-throwing callback in the same place in the event loop) fails in 6 to 8 of 10 runs on either build.
  • 370 upstream test-fs-* and test-trace-events-* files, each run as bun <file>: 359 exit 0 on both builds, and the same 11 do not on both.
  • test/aot/library-sweeps/names-lengths-every-function-walk-finds.js and test/aot/every-library-module-runs-under-the-checks.test.ts pass. bun src/codegen/check-types-of-modules.ts reports no new error. test/internal/source-lints passes.
  • A wider sweep ran on builds of an earlier form of this change and of main 2596ac5: 1,110 argument lists over all 45 function exports, also with null and with a wrong type in each place. 0 outcomes differed. The declared bodies are the same text now.
  • Not run: macOS and Windows.

…'s length

`asyncWrap` gave each export the `length` of its native operation. That
number counts the callback of the callback form, and it is 0 while a
`node.fs` trace category is on. 23 of the 32 functions that Node also
exports reported a `length` that Node does not (24 on macOS).

- 16 functions (17 on macOS) and `readFile` declare the parameters that Node
  declares and call the native operation by name, as `rm` and `rmdir` do.
- `chown`, `lchown`, `utimes`, `lutimes` and the ten functions that only Bun
  exports keep `asyncWrap`, which passes on the caller's exact arguments. It
  takes the length as an argument and no longer reads it from the operation.
- `writeFile`, `appendFile`, `open` and `opendir` keep their text and get a
  stated length.
- `FileHandle.prototype.writeFile` declares `options` with no default, so its
  length is 2.
@robobun

robobun commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 12:05 AM PT - Nov 11th, 2026

❌ @robobun, your commit 917e717 has some failures in Build #140055 (All Failures)


🧪   To try this PR locally:

bunx bun-pr 48555

That installs a local version of the PR into your bun-48555 executable, so you can run:

bun-48555 --bun

@robobun

robobun commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

Status

Reproduced on main (89452d2, Linux x64) against Node v26.12.1:

bun -p 'require("node:fs/promises").mkdir.length'    # 3
node -p 'require("node:fs/promises").mkdir.length'   # 2

The script in the Notes of the PR body prints the length of every function. On main, 23 of the 32 functions that Node also exports differ from Node, and 25 differ under --trace-event-categories node.fs. With this branch, none differs in either run.

PR: #48555

@coderabbitai

coderabbitai Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Pre-merge checks: Inconclusive 2

Failed Failed checks: 2 inconclusive

Check name Status Explanation Resolution
Title check Pending This assessment has not completed. The check will be retried on the next pre-merge check run.
Description check Pending This assessment has not completed. The check will be retried on the next pre-merge check run.
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline finding, I checked the main risk of the declared wrappers — forwarding an explicit undefined where the caller passed nothing — against each native parser in src/runtime/node/node_fs.rs for the 18 functions moved off asyncWrap (chmod/fchmod use unwrap_or(UNDEFINED), access/copyFile go through from_js_number_only which maps undefined to the default, truncate uses next_eat().unwrap_or(UNDEFINED), stat/lstat/statfs gate on is_object(), mkdtemp/readlink/realpath use parse_encoding_arg, symlink checks is_undefined_or_null): none distinguishes missing from undefined, so no error text or default changes for those. Also confirmed FileHandle.prototype.writeFile still defaults encoding to utf8 with the = "utf8" default removed, and there is no second copy of fs.promises under src/js/ahead-of-time to keep in sync. I did not run the new tests; the per-argument sync-vs-promise comparison in the "arguments" block is the check that would catch what reading cannot.

Extended reasoning...

The change replaces asyncWrap with declared async wrappers in src/js/node/fs.promises.ts so node:fs/promises functions report Node's .length, and adds length/argument-forwarding tests. It touches no security-sensitive surface. The one confirmed inline finding (bun:test treating higher-length bound functions as done-callback tests) is a disclosed behavior change rather than a code defect, and the remaining risk I examined — native parsers reacting differently to an explicit undefined — was ruled out by reading each parser.

Comment thread src/js/node/fs.promises.ts Resolved

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant