Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions packages/bun-types/s3.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,9 @@ declare module "bun" {
* @param begin Starting byte offset
* @param end Ending byte offset (exclusive)
* @param contentType Optional MIME type for the slice
* @returns A new S3File representing the specified range
* @returns A new S3File representing the specified range. A slice of a
* zero-length slice (`file.slice(0, 0).slice()`) is an empty `Blob` with
* no `name`, `bucket` or `presign`.
*
* @example
* ```ts
Expand Down Expand Up @@ -618,7 +620,6 @@ declare module "bun" {
* }
* ```
*/
readonly readable: ReadableStream<Uint8Array<ArrayBuffer>>;
stream(): ReadableStream<Uint8Array<ArrayBuffer>>;

/**
Comment on lines 620 to 625

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.

🟡 (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.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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; // undefined

I did not apply either fix here. The reasons:

  • Keep the S3 store in the zero-size arm of Blob::slice. The result is then an S3File of size 0, as file.slice(0, 0) is today. A read of that object goes to the network: await file.slice(0, 0).text() sends Range: bytes=0-0 and returns the first byte of the object (S3: a zero-length slice sends Range: bytes=N-N and reads one byte #43929). Today await hole.text() is "" with no request. So this trades a wrong name for wrong bytes until the zero-length read changes too.
  • Carry only the name. name becomes true and presign stays undefined, so slice(): S3File is still wrong for the value. That arm also drops the name of a File, of a Bun.file() and of a Blob with a name set: new File(["abc"], "a.txt").slice(0, 1).name is "a.txt", and .slice(0, 0).slice().name is undefined. So the change is about Blob::slice for every kind of blob. It needs a native build and run-time tests of its own.
  • Type the result as S3File | Blob. Then file.slice(0, 1024).presign(), .bucket and .name fail 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.

Expand All @@ -630,7 +631,7 @@ declare module "bun" {
* console.log(file.name); // "folder/image.jpg"
* ```
*/
readonly name?: string;
readonly name: string;

/**
* The bucket name containing the file.
Expand Down
2 changes: 1 addition & 1 deletion test/integration/bun-types/fixture/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ fetch("https://example.com", { body: Bun.file("hey").stream() });
fetch("https://example.com", { body: new Response("bun").body });
fetch("https://example.com", { body: Bun.s3.file("hey") });
fetch("https://example.com", { body: Bun.s3.file("hey").stream() });
// @ts-expect-error - an S3File has no `readable`, use `stream()`
fetch("https://example.com", { body: Bun.s3.file("hey").readable });

async function* asyncGenerator() {
yield "chunk1";
yield "chunk2";
Expand Down
6 changes: 5 additions & 1 deletion test/integration/bun-types/fixture/s3.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
import { s3 } from "bun";
import { expectType } from "./utilities";

async function doFileOps(file: Bun.S3File) {
console.log(file.bucket);
console.log(file.presign());
console.log(file.presign({ expiresIn: 1, method: "PUT" }));
console.log(file.type);

expectType(file.name).is<string>();
new File([await file.bytes()], file.name, { type: file.type });

await file.json();
await file.arrayBuffer();
await file.delete();
await file.formData();

for await (const chunk of file.readable) {
for await (const chunk of file.stream()) {
console.log(chunk);
}
}
Expand Down
Loading