Skip to content

fix(types): correct FormData iterator return types to include File #27195

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.

Sign up for GitHub

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

Closed
robobun wants to merge 2 commits into from
Closed

fix(types): correct FormData iterator return types to include File #27195

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6abf8fd
fix(types): correct FormData iterator return types to include File
Feb 19, 2026
190312e
Merge branch 'main' into claude/fix-formdata-iterator-types
alii Apr 23, 2026
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
4 changes: 2 additions & 2 deletions packages/bun-types/globals.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1680,8 +1680,8 @@
set(name: string, blobValue: Blob, filename?: string): void;
forEach(callbackfn: (value: Bun.FormDataEntryValue, key: string, parent: FormData) => void, thisArg?: any): void;
keys(): IterableIterator<string>;
values(): IterableIterator<string>;
entries(): IterableIterator<[string, string]>;
values(): IterableIterator<Bun.FormDataEntryValue>;
entries(): IterableIterator<[string, Bun.FormDataEntryValue]>;

Check notice on line 1684 in packages/bun-types/globals.d.ts

View check run for this annotation

Claude / Claude Code Review

Missing [Symbol.iterator]() declaration on FormData 

While you're fixing the iterator return types here, consider also adding `[Symbol.iterator](): IterableIterator<[string, Bun.FormDataEntryValue]>;` — the runtime aliases `Symbol.iterator` to `entries()` (JSDOMFormData.cpp), but bun-types doesn't declare it, so without `lib.dom` `for (const [k, v] of fd)` fails to type-check. This is a pre-existing gap, not something this PR introduced, but it's a one-line addition adjacent to the lines you're already touching and falls under the same "FormData i
Comment on lines +1683 to +1684

@claude claude Bot Apr 23, 2026

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.

🟣 While you're fixing the iterator return types here, consider also adding [Symbol.iterator](): IterableIterator<[string, Bun.FormDataEntryValue]>; — the runtime aliases Symbol.iterator to entries() (JSDOMFormData.cpp), but bun-types doesn't declare it, so without lib.dom for (const [k, v] of fd) fails to type-check. This is a pre-existing gap, not something this PR introduced, but it's a one-line addition adjacent to the lines you're already touching and falls under the same "FormData iterator return types" concern.

Extended reasoning...

What's missing

The interface FormData at packages/bun-types/globals.d.ts:1664-1685 declares keys(), values(), and entries(), but has no [Symbol.iterator]() declaration. Per the WHATWG spec, FormData is iterable and its default iterator yields the same [string, FormDataEntryValue] pairs as entries(). Bun's runtime implements exactly that — JSDOMFormData::finishCreation does putDirect(vm, vm.propertyNames->iteratorSymbol, getDirect(vm, builtinNames.entriesPublicName())), aliasing @@iterator to the entries function.

How it manifests

Without lib.dom loaded (the default for Bun projects, which use "lib": ["ESNext"]), TypeScript sees only the bun-types FormData interface. Since that interface has no [Symbol.iterator](), code like:

const fd = new FormData();
for (const [k, v] of fd) { /* ... */ }

fails with TS2488 ("Type 'FormData' must have a 'Symbol.iterator' method that returns an iterator"), and [...fd] likewise fails. The runtime supports this perfectly fine — only the types are missing.

Why nothing else covers it

A grep across packages/bun-types shows this is the only interface FormData declaration, and Symbol.iterator is declared for CookieMap and sqlite.Statement but not FormData. bun-types depends only on @types/node (which provides no global FormData) and imports undici-types only for EventSource, so there's no other source of a [Symbol.iterator] for FormData when lib.dom is absent. The existing type fixture at test/integration/bun-types/fixture/globals.ts tests entries()/keys()/values() but not for...of, which is why this gap hasn't been caught.

Step-by-step proof

  1. Project uses @types/bun with "lib": ["ESNext"] (no lib.dom) — the recommended Bun setup.
  2. TypeScript resolves FormData to packages/bun-types/globals.d.ts:1664-1685.
  3. That interface has entries(): IterableIterator<[string, Bun.FormDataEntryValue]> (after this PR) but no [Symbol.iterator]().
  4. User writes for (const [name, value] of fd) { ... }.
  5. tsc emits TS2488 because FormData is not declared iterable.
  6. At runtime the same code works, since the C++ prototype installs @@iterator = entries.

Relevance to this PR

This is pre-existing — the PR didn't introduce or worsen it. But the PR's stated goal is to "correct FormData iterator return types to include File", and the default iterator is the most common way people iterate FormData. Fixing values()/entries() while leaving for...of un-typed is an incomplete fix for the same conceptual issue, and the addition is one line right next to the lines being changed.

Suggested fix

entries(): IterableIterator<[string, Bun.FormDataEntryValue]>;
[Symbol.iterator](): IterableIterator<[string, Bun.FormDataEntryValue]>;

This matches how lib.dom.iterable.d.ts declares it, so when lib.dom is loaded the merged interface remains consistent (both declare the same signature shape). Not a blocker — just a suggestion for completeness while you're in this exact spot.

}
declare var FormData: Bun.__internal.UseLibDomIfAvailable<"FormData", { prototype: FormData; new (): FormData }>;

Expand Down
26 changes: 26 additions & 0 deletions test/regression/issue/27194.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { expect, test } from "bun:test";

test("FormData.values() returns File objects, not just strings", () => {
const fd = new FormData();
const file = new File(["content"], "test.txt", { type: "text/plain" });
fd.append("textField", "hello");
fd.append("fileField", file);

const values = [...fd.values()];
expect(values).toHaveLength(2);
expect(values[0]).toBe("hello");
expect(values[1]).toBeInstanceOf(File);
});

Check warning on line 13 in test/regression/issue/27194.test.ts

View check run for this annotation

Claude / Claude Code Review

Runtime test does not guard the type-only fix; wrong test location 

This runtime test doesn't guard the actual change in this PR — the PR only edits `.d.ts` declarations, and `FormData.values()`/`entries()` already returned `File` at runtime, so these assertions pass on system Bun before the fix (violating the `USE_SYSTEM_BUN=1` validity rule in CLAUDE.md). Per CLAUDE.md, `.d.ts` changes should be covered by a type-level assertion in `test/integration/bun-types/fixture/globals.ts` (which already exercises `FormData`), and since #27194 was never correct in any re
Comment on lines +1 to +13

@claude claude Bot Apr 23, 2026

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.

🟡 This runtime test doesn't guard the actual change in this PR — the PR only edits .d.ts declarations, and FormData.values()/entries() already returned File at runtime, so these assertions pass on system Bun before the fix (violating the USE_SYSTEM_BUN=1 validity rule in CLAUDE.md). Per CLAUDE.md, .d.ts changes should be covered by a type-level assertion in test/integration/bun-types/fixture/globals.ts (which already exercises FormData), and since #27194 was never correct in any release it isn't a true regression — so this file shouldn't live under test/regression/issue/ either.

Extended reasoning...

What's wrong

This PR's only functional change is to two type annotations in packages/bun-types/globals.d.ts. The accompanying test at test/regression/issue/27194.test.ts is a runtime test (expect(values[1]).toBeInstanceOf(File)), but the runtime behavior it asserts was already correct before this PR — only the .d.ts types were wrong. Consequently the test does not regression-guard the change: if someone reverted values(): IterableIterator<Bun.FormDataEntryValue> back to IterableIterator<string>, this test would still pass.

Why existing guidance flags this

Two repo conventions apply directly:

  1. Test validity — CLAUDE.md:130 states: "CRITICAL: Verify your test fails with USE_SYSTEM_BUN=1 bun test <file> and passes with bun bd test <file>. Your test is NOT VALID if it passes with USE_SYSTEM_BUN=1." This test passes on system Bun (the C++ DOMFormData has always yielded File for blob entries), so it provides zero protection for the diff.
  2. Where .d.ts tests belong — CLAUDE.md:37-43 says edits under packages/bun-types/**/*.d.ts should be tested via bun test test/integration/bun-types/bun-types.test.ts, which packs the .d.ts files and runs tsc against fixtures. That's the only test that would actually fail if the type declarations regressed.

Separately, CLAUDE.md:66 and test/CLAUDE.md:153 reserve test/regression/issue/${issueNumber}.test.ts for true regressions (worked in a prior release, then broke). #27194's types were never correct, so even if a runtime test were kept it would belong in test/js/web/html/FormData.test.ts, not a new regression file.

Step-by-step proof

  1. Before this PR, globals.d.ts declares values(): IterableIterator<string>, but JSDOMFormData's iterator yields the underlying FormDataEntryValue variant (String | Blob).
  2. Run USE_SYSTEM_BUN=1 bun test test/regression/issue/27194.test.ts against a released Bun → values[1] is a File, toBeInstanceOf(File) passes. ✅ Test green without the fix.
  3. Now apply this PR, then revert just the two .d.ts lines → bun bd test test/regression/issue/27194.test.ts still passes. ✅ Test green with the fix reverted.
  4. Conclusion: the test cannot detect the absence of the fix in either direction.

Suggested fix

Replace the runtime test with a type-level assertion in test/integration/bun-types/fixture/globals.ts (lines ~221-235 already exercise FormData), e.g.:

import { expectType } from "./utilities";
// ...
const fd = new FormData();
for (const v of fd.values()) expectType<Bun.FormDataEntryValue>(v);
for (const [k, v] of fd.entries()) {
  expectType<string>(k);
  expectType<Bun.FormDataEntryValue>(v);
}

This will fail tsc if the iterator types ever regress to string. If runtime coverage is also desired, add it to the existing test/js/web/html/FormData.test.ts rather than a new test/regression/issue/ file.


test("FormData.entries() returns File objects in value position", () => {
const fd = new FormData();
const file = new File(["content"], "test.txt", { type: "text/plain" });
fd.append("textField", "hello");
fd.append("fileField", file);

const entries = [...fd.entries()];
expect(entries).toHaveLength(2);
expect(entries[0]).toEqual(["textField", "hello"]);
expect(entries[1][0]).toBe("fileField");
expect(entries[1][1]).toBeInstanceOf(File);
});
Loading