Repository navigation
Conversation
Bun.write(s3file, input, options) reads `options` as S3 options and takes a Response or a Request as input. The declarations took neither, so both calls were TS2769. - Bun.write: one added signature for an S3File destination, after the two file signatures for a Response or a Request. A file write tries no extra signature. The options are S3Options plus the two file options, which the runtime also parses for an S3 destination. - S3File.write and both S3Client.write: the data union gains ReadableStream, which the runtime takes. - Tests in s3.test.ts run what the new JSDoc says about the options. - Remove two casts in fetch-backpressure.test.ts that the missing signature made necessary, and correct a stale comment in s3.test.ts.
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
S3 write signatures and type coverage packages/bun-types/bun.d.ts, packages/bun-types/s3.d.ts, test/integration/bun-types/* |
Bun.write gains an S3File overload, and S3 write methods accept ReadableStream. Type tests cover supported inputs, options, and compatibility with exactOptionalPropertyTypes. |
Upload option behavior tests test/js/bun/s3/s3.test.ts, test/js/web/fetch/fetch-backpressure.test.ts |
Tests cover per-write ACL and content disposition, content type handling, and partSize behavior for stream and BunFile inputs. Fetch backpressure tests remove casts from S3 file calls. |
Suggested reviewers: jarred-sumner
Priority:
Merge Risk: 
a32e8
-
The S3 write typings align with the inspected upload path, and no issue remains that should block merging after normal checks.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly identifies both main changes: the S3File overload for Bun.write() and ReadableStream support for S3 write() data. Description check The description explains the problem, fix, scope, runtime behavior, tests, verification results, and known limitations. It does not use the template headings exactly, but it provides the required info… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description explains the problem, fix, scope, runtime behavior, tests, verification results, and known limitations. It does not use the template headings exactly, but it provides the required information in equivalent sections.
-
- 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.
|
Updated 3:45 PM PT - Oct 25th, 2026
✅ @robobun, your commit a32e8be447fa2d702d72890df427bffe26baef4a passed in 🧪 To try this PR locally: bunx bun-pr 46923That installs a local version of the PR into your bun-46923 --bun |
|
Status How I reproduced it, with the declarations of main (179664e): import { S3Client } from "bun";
declare const client: S3Client;
await Bun.write(client.file("k"), "x", { acl: "private" }); // error TS2769: No overload matches this call.
await Bun.write(client.file("k"), new Response("x")); // error TS2769: No overload matches this call.
await client.file("k").write(new ReadableStream()); // error TS2345The three calls upload at runtime against
PR: #46923 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
The runtime does not apply a `type` given to Bun.write(s3file, x,
options): the content type comes from the S3File. The signature now
takes `Omit<S3Options, "type">`, so a literal `{ type }` stays a
compile error there, as it is on main. S3File.write() and the `type`
option of file() do apply it, and the JSDoc points to them.
Add a type test with exactOptionalPropertyTypes for the spelling of
`mode` and `createPath` on that signature: a function typed as
Bun.write that takes the file options must still compile.
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.
Commit 6f9840e addresses both points from my earlier review: the options type is now Omit<S3Options, "type">, and the new exactOptionalPropertyTypes typeTest in test/integration/bun-types/bun-types.test.ts shows why mode/createPath are spelled without | undefined (each overload's options must stay assignable to a typeof Bun.write mock's { mode?; createPath? }). Beyond the inline nit, I also checked that passing a Bun.S3Options variable carrying type still compiles (no excess-property check on non-literals) -- that is the designed escape hatch and the runtime test "does not apply a type of the call" pins its behavior -- and that the new overload sits after the Response file signatures so existing Bun.write(path, response) bindings are unchanged. Since packages/bun-types/ is CODEOWNER-owned, a maintainer look at the overload shape is still worthwhile.
Extended reasoning...
The change is declarations-only: a fourth Bun.write overload for an S3File destination in packages/bun-types/bun.d.ts, ReadableStream added to the data unions of the three S3 write() methods in packages/bun-types/s3.d.ts, a type fixture, a typeTest, three runtime tests in test/js/bun/s3/s3.test.ts, and removal of two as any casts. It touches no security-sensitive surface. The second commit resolved both of my prior inline findings. Approval is withheld because *.d.ts and packages/bun-types/ are CODEOWNER-owned and the overload design (position, deliberate deviation from the | undefined convention) is an API-surface decision for a maintainer; the one remaining finding is a test nit posted inline.
`expect.not.stringContaining("-")` also passes for a value that is not a
string, so the two one-part rows passed with a missing ETag. Each row
now matches the shape of its ETag.
Problem
Bun.write(s3file, input, options)reads S3 options and takes aResponseorRequest. The declarations take neither:Bun.write(client.file("k"), "x", { acl: "private" })iserror TS2769: No overload matches this call(packages/bun-types/bun.d.ts:2151). Fix every type error intest/#44614 lists both gaps.write()methods take aReadableStream, which theirdataunions lack (packages/bun-types/s3.d.ts:716).Fix
Bun.write(destination: S3File, input, options?), after theResponsefile signatures, so a file write tries no extra signature. Its options areS3Optionswithouttype, plus the two file options. The threedataunions gainReadableStream.src/runtime/webcore/Blob.rs:5026). Tests ins3.test.tsrun each sentence of the new JSDoc.test/integration/bun-types/bun-types.test.ts(25 pass, 10 fail on main's declarations) andtest/js/bun/s3/s3.test.ts. Thetscprojects oflint.ymlandtest: 0 new diagnostics.Bun.writeas three signatures (Notes).Background
Bun.writehas one declaration for each kind of call, and TypeScript takes the first that matches.S3Fileis whatclient.file(key)returns.s3file.write(input, options)is the same native write, typed already.S3Filefrom the first signature, so an editor completes S3 options in{ }. That needs a copy of the signature and changes no compile result.Downsides
Bun.write(s3file, x, { })still completescreatePath, mode, as on main.bun checkspends +284 instructions (+1.1%) for eachBun.writecall.Parameters<S3File["write"]>[0]used as the old union. This repository has none.Notes
What the added signature takes
input: the input of the file signature, plusArrayBufferView(aDataView),ResponseandRequest. Each kind was run with options againsttest/packages/s3-server.options:S3Optionswithouttype, and the two file options. The runtime does not apply atypeof this call, so a literal{ type }stays a compile error, as on main. A variable typedS3Optionsstill fits. The runtime parsesmodeandcreatePathfrom the same object for each destination. For S3,modeis range-checked and not used, andcreatePathis not used. Both compile for anS3Fileon main. With them in the type, the options of the new signature extend the file options. So a function typedtypeof Bun.writethat declaresoptions?: { createPath?: boolean }still compiles. TypeScript also offers no S3 key while a file write has a half-typed key. They are spelled without| undefined, like the file signatures: with it, both effects are lost underexactOptionalPropertyTypes. A type test with that flag holds the spelling.Responsesignatures, eachBun.write(path, response)would try and reject it. Last, it would change the error text of each call that matches no signature, because TypeScript prints the error of the last one.What the JSDoc says about the options, and the test for it
S3Filefor that write only. Test:applies an option of the call to that write only.optionstakes notype. The content type is thetypeof the file, or else the type of the extension of its key.s3file.write(x, { type })and thetypeoption offile()do set it. Test:does not apply a type of the call.partSize,queueSizeandretryof the call are applied to a stream input. For aBunFileor anS3Fileinput, those of the destination are used. Test:applies partSize of the call to a stream input, not to a BunFile input. s3: add tagging option and route write headers through one carrier #46746 changes theBunFilecase. It then changes this sentence and this test.These tests describe main's runtime, so they pass without this PR. They fail when the runtime changes under the JSDoc. Runtime checks ran on a debug build of this branch and on canary a8fcd31.
Measurements, main 179664e against this PR
getSignatureApplicabilityError): no change for a call that compiles on main and has noanyargument (6 file shapes,S3File,BunFile | S3File).anydestination 6 to 7. A call that matches no signature 11 to 13. AnS3Filecall that is TS2769 on main 11 to 4.--extendedDiagnostics, slope 1000 to 3000 calls): -0.03 to +0.00 for each shape that compiles on main. A call that is TS2769 on main: 362.47 to 21.17.anydestination +244.bun checkuser instructions for each call site (perf_event_open, minimum of 5 interleaved runs, slope 2000 to 4000 calls, spread 53 to 150): file write with a literal path +194 (+0.7%), with astringpath +284 (+1.1%). Response-body file write +266 (+0.8%).S3Filedestination +260 (+1.0%).anydestination +2,308 (+6.5%). AnS3Filecall that is TS2769 on main: 2,670,471 to 81,224.Bun.writecall sites of thetestproject (2,023 sites): candidates tried 2,150 to 2,164 (+14). 2,008 sites unchanged. +1 at 8 sites with ananyargument, +2 at 6 calls that match no signature, -6 at 1 site.lint.yml.testproject: 1 gone (test/js/bun/s3/s3-stream-stored-error-fixture.ts:79), 0 new, 0 new unused@ts-expect-error.typeof Bun.writewith each parameter annotatedBun.writepicked by indexParameters<..>of an S3write()read as the old union// @ts-expect-errorabove a call that now compilesBun.write(s3file, "x", { })staycreatePath, mode. After one S3 key, or with aResponseinput, the S3 keys are listed. With a half-typed key the list iscreatePath, as on main, for each destination. Hover onBun.write(s3file, "x", { acl })shows the new signature. The error text of a call that matches no signature does not change.bun.d.ts+1,739 bytes,s3.d.ts+90 bytes. No native file changes.Not in this PR, with the reason
S3Client.write,presign,unlinkanddeletetake anS3Filefor the path at runtime (src/runtime/webcore/S3File.rs:112). They staypath: string. No document, test or caller uses that form, and a declaration makes it supported API. It needs a maintainer's decision.S3Filein the first signature.Bun.write(s3file, "x")still binds the file signature, so the editor shows the file options and the file documentation for it. To change that, the first signature must loseS3File, and a copy of it must keepBunFile | S3Filedestinations compiling.S3File, one data type). It changes what each call binds. Bun.write: applymodefor string, buffer, Blob and stream sources on POSIX #32885, docs, bun-types: describe how Bun.write and FileSink treat an existing file, accept a file descriptor number #41745 and Bun.write: accept Bun.Image and reject unsupported input with ERR_INVALID_ARG_TYPE #44006 edit the same block.write()methods. The runtime drops each part that is aBunFileor anS3File:s3file.write(["a", Bun.file(p)])stores"a"(Blob constructor ignores File parts if more than one part is given #25851, Blob: read file-backed parts in multi-part new Blob([...]) #33600). The fixture keeps the array form an error there.Bun.write("s3://bucket/key", x, s3Options). As3://${string}destination type would miss each URL that astringvariable holds, which is the form attest/js/bun/s3/s3.test.ts:1618.Bun.file("s3://..")is declaredBunFile.BunFile.write()keeps its narrowerdataunion and its{ highWaterMark }options (docs, bun-types: describe how Bun.write and FileSink treat an existing file, accept a file descriptor number #41745 rewrites them).S3File.delete()is declared without options, and the runtime reads them (s3: add a timeout option and a per-operation signal option #45695 declares them).Bun.write(BunFile | S3File, Response | Request)and S3 options on a union destination stay errors.Self-review
The first version of this branch was wider. It also removed
S3Filefrom the first signature and added a copy of that signature for union destinations, with a pinned order. It typed the four statics and used a sharedBun.__internaldata type. The review changed it as follows:partSizefor aBunFileinput. It now states the exception, with a test.__internalis not for a union that callers pass. The unions are inline, and fixture checks tie them.acl: "public-read". It usesstorageClass.Not taken: the rewrite to three signatures, for the reason above. Also not taken: one concern asked to keep the reordered signatures for the editor. That is the second item of the list above.
After the code review of the first push:
S3Options, so a literal{ type }compiled and was not applied. They now leavetypeout.| undefinedonmodeandcreatePath. Not taken: withexactOptionalPropertyTypes, a function typedtypeof Bun.writethat takes the file options then fails with TS2322. The new type test shows it.Other open PRs on these lines
S3Filesignature, second, withS3RequestOptionsand withoutResponse | Request. It can change the options type of this one.Imageto the same unions. The fixture fails if it is added to the file signature only.tagging) and bun-types: declareS3File.nameasstringand removeS3File.readable#46754 (S3File.name) edit the same fixture.test/js/bun/s3/s3-stream-stored-error-fixture.ts:79. With this PR that call compiles without it.Demand. No user reported the type error. The record is the gap list of #44614, the three-argument call in the repro of #42722, and two casts in
test/js/web/fetch/fetch-backpressure.test.ts, which this PR removes. The other four casts that #44614 left for S3 stay: they are fors3://destinations.Suites run.
test/integration/bun-types/bun-types.test.ts. Under a debug build:test/js/bun/s3/s3.test.ts(310 pass, 270 skipped for credentials) and the S3 block oftest/js/web/fetch/fetch-backpressure.test.ts(8 pass).tsc --noEmit -pforscripts/build,scripts,src/js,src,src/runtime/bake,testandpackages/bun-types.bun fmt --check.no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/web/fetch/fetch-backpressure.test.ts