Repository navigation
Conversation
Two members of `interface S3File` did not match the run time. #15740 took both from BunFile. `name` was `name?: string`. The name of an S3File is always a string: the path given to file(). `new File([bytes], s3file.name)` failed under strict with TS2345. `readable` was declared, and no S3File has it: `"readable" in s3file` is false. Code that reads it compiles and gets `undefined`. The declaration is gone, so that code now fails with TS2339. `stream()` replaces it. #20879 removed the same member from BunFile. One case stays as it is: slice() on a blob whose size is 0 returns an empty plain Blob, not an S3File. That Blob has no name, bucket or presign. The JSDoc of slice() now says so. BunFile.name stays optional: a file opened by file descriptor has no name.
|
Updated 10:49 AM PT - Oct 24th, 2026
✅ @robobun, your commit 31290b90967c6c023c294449e348ce1879cafa82 passed in 🧪 To try this PR locally: bunx bun-pr 46754That installs a local version of the PR into your bun-46754 --bun |
|
Status: ready for review. The checks that test this diff pass: Buildkite (build 132055, state at 17:15 UTC on Oct 24): 179 of 181 jobs pass and no test is red. The two jobs left are How to reproduce (a release build of
|
There was a problem hiding this comment.
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 also swept docs/ and packages/bun-types/ for remaining references to S3File.readable — none exist outside the fixture now marked @ ts-expect-error, so the removal leaves no stale JSDoc or docs behind. The @ ts-expect-error line in fetch.ts replaces a blank line, so the diagnostics pinned at fetch.ts:25/:33/:168 in bun-types.test.ts keep their line numbers.
Extended reasoning...
The change edits only TypeScript declarations in packages/bun-types/s3.d.ts (drops readable, tightens name? to name, extends the slice() JSDoc) plus the two type-check fixtures; no runtime or security-sensitive surface is touched. The inline finding on the name: string declaration versus the empty-slice Blob case is the open question for a human, and the ruled-out items above are the only extra checks worth recording.
| * } | ||
| * ``` | ||
| */ | ||
| readonly readable: ReadableStream<Uint8Array<ArrayBuffer>>; | ||
| stream(): ReadableStream<Uint8Array<ArrayBuffer>>; | ||
|
|
||
| /** |
There was a problem hiding this comment.
🟡 (optional) Users who slice a zero-size S3 slice now get name typed string while the runtime returns undefined, so code TypeScript accepts throws or builds a File named "undefined". slice() on a blob whose size is 0 returns a plain empty Blob with no store (src/runtime/webcore/Blob.rs:1755), and its name getter yields undefined (Blob.rs:1845), yet the overloads at s3.d.ts:535-537 still return S3File with readonly name: string (s3.d.ts:634). Fix: make the declaration true on that path too, e.g. have the zero-size arm of Blob::slice carry the parent's name (and S3 store) into the result, or type the overloads so the empty-slice result is not S3File; the JSDoc note alone leaves the type lying. The same hole opens after resolve_size() zeroes a live S3 blob (Blob.rs:2067).
Why this was flagged
A caller does const hole = s3.file("k").slice(0, 0).slice() (or slices an S3 blob after something called resolve_size() on it, Blob.rs:2067, which sets an S3 blob's size to 0). Blob::slice at src/runtime/webcore/Blob.rs:1755 sees self.size.get() == 0 and returns Blob::init_empty, a plain Blob with no store. Its get_name (Blob.rs:1842-1846) finds no name and no store_path, so it returns JSValue::UNDEFINED. The declarations say slice(): S3File (s3.d.ts:535-537) and, after this diff, readonly name: string (s3.d.ts:634), so hole.name.length or new File([..], hole.name) compiles with no narrowing. On the base branch name?: string forced a ?? / ! guard, so the undefined was visible to the type checker; after merge the user gets a runtime TypeError or a File named "undefined" that tsc said was impossible. The only safeguard is the new JSDoc sentence at s3.d.ts:510-512, which the compiler does not enforce and which also omits the resolve_size() trigger.
Verification: slice() on an S3 blob whose size is 0 (s3.file("k").slice(0, 0).slice()) yields no name. Blob::slice (src/runtime/webcore/Blob.rs:1755-1760) returns a plain Blob with no store, so get_name (Blob.rs:1842-1846) returns undefined. s3.d.ts:535-537 still declares slice(...): S3File and this PR changes s3.d.ts:634 to readonly name: string, so TypeScript now accepts hole.name.length (runtime TypeError) where the base forced narrowing.
There was a problem hiding this comment.
This is true, and the PR states it (Downsides, second bullet, and "The slice() exception" under Notes). On canary 1.4.4-canary.1+a8fcd319b:
const hole = s3.file("k").slice(0, 0).slice(); // [object Blob]
hole.name; // undefined
hole.presign; // undefinedI did not apply either fix here. The reasons:
- Keep the S3 store in the zero-size arm of
Blob::slice. The result is then anS3Fileof size 0, asfile.slice(0, 0)is today. A read of that object goes to the network:await file.slice(0, 0).text()sendsRange: bytes=0-0and returns the first byte of the object (S3: a zero-length slice sendsRange: bytes=N-Nand reads one byte #43929). Todayawait hole.text()is""with no request. So this trades a wrongnamefor wrong bytes until the zero-length read changes too. - Carry only the name.
namebecomes true andpresignstaysundefined, soslice(): S3Fileis still wrong for the value. That arm also drops the name of aFile, of aBun.file()and of aBlobwith a name set:new File(["abc"], "a.txt").slice(0, 1).nameis"a.txt", and.slice(0, 0).slice().nameisundefined. So the change is aboutBlob::slicefor every kind of blob. It needs a native build and run-time tests of its own. - Type the result as
S3File | Blob. Thenfile.slice(0, 1024).presign(),.bucketand.namefail for every caller:TS2339: Property 'presign' does not exist on type 'Blob | S3File'.
The JSDoc leaves out the resolve_size() route on purpose. #46653 changes that arm (store::DataTag::S3 => {}), so the JSDoc describes only the part that stays: the slice of a zero-length slice.
The decision for a maintainer: take name: string with this one documented exception, or keep name?: string until the run-time hole is closed. If the second is preferred, I will reduce this PR to the removal of readable.
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
S3File contract and fixture checks packages/bun-types/s3.d.ts, test/integration/bun-types/fixture/fetch.ts, test/integration/bun-types/fixture/s3.ts |
The declaration requires S3File.name and removes readable. Its slice documentation states that a zero-length slice produces an empty Blob without name, bucket, or presign. The fixtures check the name type, mark readable as unavailable, construct a File, and iterate over stream(). |
Priority:
Merge Risk: 
Minimal · up to 31290
-
This change makes
S3File.namerequired, removes thereadabledeclaration that does not exist at runtime, and documents the zero-length slice behavior. No concrete merge-blocking risk remains. The documented zero-length slice edge case is a known minor typing limitation.
Pre-merge checks: 
4
-
Full details: Title check
-
Explanation
The title names the two main declaration changes: S3File.name becomes string and S3File.readable is removed. Both match the changeset in packages/bun-types/s3.d.ts and the bun-types fixtures. It is specific and concise, and it fits the 'bun-types:' prefix convention used in this repo.
Full details: Description check
-
Explanation
The description follows the template's two required sections. 'What does this PR do?' is covered by the Problem, Fix, and Background sections. 'How did you verify your code works?' is covered by the bun-types test run (24 of 24 pass, 10 cases fail on the old declarations), the scripts/build, scripts, src/js, src, and src/runtime/bake type checks, and the test/ tsc comparison. The description is detailed and includes reproduction steps, downsides, and history. It is long, and the Notes block is dense, but the required information is present.
-
- 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.
Comment @coderabbitai help to get the list of available commands.
Problem
S3File.nameis declaredreadonly name?: string(packages/bun-types/s3.d.ts:633). Understrict,new File([await s3file.bytes()], s3file.name)fails:TS2345: Argument of type 'string | undefined' is not assignable to parameter of type 'string'.S3File.readableis declared (s3.d.ts:621), but"readable" in s3fileisfalse.fetch(url, { method: "POST", body: s3file.readable })compiles and sends no bytes.Fix
readonly name: string. Deletereadable, as MoveReadableStream#{text,bytes,json,blob}to global #20879 did forBunFile. Both came fromBunFilein feat(s3) s3 client #15740.store_path(), which returns the S3 path (src/jsc/webcore_types.rs:420).test/integration/bun-types/fixture/s3.tsandfetch.ts(10 of 24 cases ofbun-types.test.tsfail with the old declarations). No executing test asserts the name.Bun.embeddedFilesandServerWebSocket.binaryType?. Each is a separate decision (Notes).Background
S3Fileis the type ofs3.file(path)and of itsslice(). At run time it is aBlobwhose store holds the S3 path.slice()on a blob of size 0 returns an empty plainBlob(src/runtime/webcore/Blob.rs:1755), not anS3File. This PR only documents that, in the JSDoc ofslice().slice(): a slice of an empty slice would then read from the network.Downsides
s3file.readablenow fails withTS2339. It getsundefinedtoday.stream()replaces it.file.slice(0, 0).slice().namenow compiles asstringand isundefined. A mock typedS3Filewith nonamenow fails withTS2741.Bun.file("s3://...")and aBunFile | S3Fileunion still givestring | undefined.Notes
No user reported this. It was found while #38364 was in review. The
TypeErrorof #38364 printsnew File([await s3file.bytes()], s3file.name, { type: s3file.type }), and its docs and JSDoc use a literal file name because of this declaration. #38364 needs no change. It can uses3file.nameonce this lands.Run time, on canary
1.4.4-canary.1+a8fcd319b. No network is needed forname: the endpoint is never dialed.Bun.S3Filehas 20 members.readableis the only one that no S3 file has.file()calls: 21 paths (empty,/, thes3://forms, non-ASCII, a trailing slash) through 10 forms (anS3Clientwith a bucket, one without,new S3Client(),Bun.s3, staticS3Client.file, each with and without options). 20 calls throw before anS3Fileexists. The other 190 results and 7 slices of each (none a slice of an empty slice) are 1520 objects. Each is[object S3File]with a stringname.nameafter an operation against a mock endpoint (exists,stat,text,bytes,json,stream,write,writer().end(),deleteand 11 more, for a present key, a 404 and a 403): each is a string. A read works on a copy of the blob (S3BlobDownloadTask::initcallsBlob::dupe).readable:for await (const chunk of f.readable)throws aTypeError.await new Response(f.readable).text()is"".fetch(url, { method: "POST", body: f.readable })sendscontent-length: 0. Withf.stream()the server receives the 11 bytes of the object.The
slice()exception.Blob::slicereturnsBlob::init_emptywhen the size field of the blob is 0. The size field of an S3 blob is 0 in two cases:slice(n, n), orslice(a, b)witha > b.resolve_size()ran on the live blob (Blob.rs:2067sets the size of an S3 blob to 0). Examples:structuredClone(f),postMessage(f),expect(f).toHaveLength(0). After one of these,f.slice(1, 3)is an empty plainBlob. s3: leave the size of an S3 blob unknown inresolve_size#46653 changes that arm ofresolve_size(). It does not change the zero-length slice.The result has no
name, nobucketand nopresign, soslice(): S3Filewas already wrong for it. With this PR,new File([await hole.bytes()], hole.name)compiles for it and makes aFilenamed"undefined". A search finds no open issue for the shortcut. A guard for S3 would keep the store, and thenhole.text()reads from the network asf.slice(0, 0).text()does today (Range: bytes=0-0, #43929).The setter.
namehas a setter at run time (setter: trueinBlob.bind.ts,Blob::set_name). A string renames the file.undefinedandnullare returned as given. The declarations do not publish the setter:nameisreadonlyhere, as onFileandBunFile. A direct write isTS2540. A write throughObject.assign,Reflect.setor an alias typed{ name?: string }compiles. This PR does not change the setter.Not in this PR. Each is a separate decision.
lastModifiedis present at run time and not declared. ThesizeJSDoc ats3.d.ts:479-501has no member under it.NetworkSink.stat(s3.d.ts:98) is declared and no sink has it. Since bindgen: move the stream sinks to a schema and deletegenerate-jssink.ts#46641,bun-types.test.tslists it as a known difference.slice(): S3Filehas the exception above.Bun.embeddedFilesisReadonlyArray<Blob>(bun.d.ts:4897), soblob.namein the snippets ofdocs/bundler/executables.mdxgivesTS2339. It needs a choice of element type and a docs edit.Bun.file(path).nameis always a string and staysstring | undefined.BunFileis also the type ofBun.file(fd)andBun.stdin, whosenameisundefined. BunFile should extend Web File #16850 asks forBunFileto extendFile.ServerWebSocket.binaryType?(serve.d.ts:315) is"nodebuffer"on a new socket. A change edits two*.test.tsfiles.markdown.CodeBlockMeta.language?(bun.d.ts:1757) is set on everymetathat thecodecallback gets.History. #15740 declared
interface S3File extends BunFilewithreadonly name?: stringandreadonly readable: ReadableStream, both asBunFilehad them. e532456 madeS3FileextendBlob. #18024 moved the interface tos3.d.ts. #20879 turnedBunFile.readableinto a comment. #42719 is an earlier one-token declaration fix with no issue behind it.Type-level effect, TypeScript 6.0.2.
typescript-eslint 8.72.0 now reports
f.name!(no-unnecessary-type-assertion) andf.name ?? x(no-unnecessary-condition). A// @ts-expect-errorthat only covered theundefinednow fails withTS2578.Checks run.
bun test test/integration/bun-types/bun-types.test.tswith canary a8fcd31: 24 of 24 pass. With the old declarations the fixtures gives3.ts:10:25 TS2554,s3.ts:11:34 TS2345andfetch.ts:19:1 TS2578in 10 cases: with and withoutlib.dom.d.ts, with TypeScript latest, and with TypeScript 7.1.fetch.tskeeps its 389 lines.bun-types.test.tsexpects diagnostics atfetch.ts:25,:33and:168, so the@ts-expect-errorline takes the place of a blank line.bun x tsc --noEmit -pforscripts/build,scripts,src/js(afterbun run build:types),srcandsrc/runtime/bake: no errors.tsc --noEmitintest/: the output is identical with and without this change.Blob.prototype.nameasb.any, so it cannot see this token.The bun-types files are not part of the binary, so there is no run-time or size cost.