-
-
Notifications
You must be signed in to change notification settings - Fork 414
fix: handle arrays properly in FormData #1431
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.
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
base: main
Are you sure you want to change the base?
Conversation
When FormData contains arrays (e.g., multiple file uploads), the current implementation incorrectly stringifies the entire array instead of appending each item individually. This fix: - Detects when a property is an array - Iterates through each array item - Properly appends each file/blob individually to FormData - Preserves non-file array items by JSON.stringify-ing them individually This enables proper multi-file uploads and array handling in FormData, which is essential for APIs that accept multiple attachments or files.
|
|
|
||
| for (const formItem of propertyContent) { | ||
| const isFileType = formItem instanceof Blob || formItem instanceof File; | ||
| formData.append(key, isFileType ? formItem : JSON.stringify(formItem)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const propertyContent = property instanceof Array ? property : [property]; | ||
|
|
||
| for (const formItem of propertyContent) { | ||
| const isFileType = formItem instanceof Blob || formItem instanceof File; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since File extends Blob, you can simplify this check to just formItem instanceof Blob.
- Remove redundant File instanceof check (File extends Blob) - Add explicit string type check to prevent double-stringification - Strings now passed directly to FormData instead of JSON.stringify Thanks to @lc-soft for the code review and suggestions! 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
@lc-soft Thank you for the excellent code review! I've implemented both of your suggestions:
The updated code now correctly handles:
Most importantly: The original bug fix (handling arrays in FormData for multi-file uploads) still works perfectly! 🎉 Changes pushed in commit b9da1cd |
Update test snapshots to reflect the simplified FormData handling: - Removed redundant File instanceof check - Added string type check to prevent double-stringification All 133 tests passing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
✅ Tests fixed! Updated all test snapshots to reflect the refactored FormData handling. All 133 tests now passing. Changes in commit 6e854b8:
|
|
Thank you for the PR. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
bugbot run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
|
|
||
| for (const formItem of propertyContent) { | ||
| formData.append(key, formItem instanceof Blob || typeof formItem === "string" ? formItem : JSON.stringify(formItem)); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: FormData Serialization Fails with BigInt and Undefined
The FormData serialization logic now uses JSON.stringify for non-Blob/non-string values, which introduces two issues. BigInt values now cause a TypeError. Also, undefined values are passed as the undefined primitive to formData.append(), relying on implicit browser conversion instead of the previous explicit string conversion, which may lead to unexpected behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a critical bug in FormData handling where arrays were being incorrectly stringified instead of having each item appended individually to the FormData object. This particularly affects multi-file upload endpoints and any FormData fields accepting multiple values.
Key Changes:
- Modified FormData content formatter to detect and properly iterate through arrays
- Each array item is now appended individually to FormData with the same key
- Simplified type checking logic for Blob/string vs other data types
Reviewed Changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/base/http-clients/fetch-http-client.ejs | Core fix: updated FormData formatter to handle arrays by iterating and appending each item individually |
| tests/fixtures/formdata-arrays.json | Added test fixture defining multi-file upload endpoint to validate array handling |
| tests/snapshots/simple.test.ts.snap | Updated snapshots reflecting new array handling logic across multiple test scenarios |
| tests/snapshots/extended.test.ts.snap | Updated snapshots for extended test cases |
| tests/spec/*/basic.test.ts.snap | Updated snapshots across all spec test suites to reflect the FormData array handling changes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| const propertyContent = property instanceof Array ? property : [property]; | ||
|
|
||
| for (const formItem of propertyContent) { | ||
| formData.append(key, formItem instanceof Blob || typeof formItem === "string" ? formItem : JSON.stringify(formItem)); |
Copilot
AI
Oct 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] This line is overly long and hard to read. Consider breaking the ternary operation onto multiple lines for better readability, similar to the original formatting style.
| formData.append(key, formItem instanceof Blob || typeof formItem === "string" ? formItem : JSON.stringify(formItem)); | |
| formData.append( | |
| key, | |
| formItem instanceof Blob || typeof formItem === "string" | |
| ? formItem | |
| : JSON.stringify(formItem) | |
| ); |
Copilot uses AI. Check for mistakes.
| const propertyContent = property instanceof Array ? property : [property]; | ||
|
|
||
| for (const formItem of propertyContent) { | ||
| formData.append(key, formItem instanceof Blob || typeof formItem === "string" ? formItem : JSON.stringify(formItem)); |
Copilot
AI
Oct 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition formItem instanceof Blob || typeof formItem === 'string' does not handle File objects correctly. File extends Blob, but numbers, booleans, and other primitive types will be JSON.stringify'd instead of being converted to strings. This changes behavior from the original code which used template literal coercion (${property}) for primitives.
| formData.append(key, formItem instanceof Blob || typeof formItem === "string" ? formItem : JSON.stringify(formItem)); | |
| formData.append(key, formItem instanceof Blob ? formItem : `${formItem}`); |
Copilot uses AI. Check for mistakes.


Description
This PR fixes an issue with FormData handling when arrays are involved, particularly for multi-file uploads.
The Problem
When an API endpoint accepts an array of files (or any array in FormData), the current implementation incorrectly converts the entire array to a string representation instead of properly appending each item individually to the FormData object.
Example scenario:
The Solution
This fix modifies the FormData content formatter to:
Changes Made
templates/base/http-clients/fetch-http-client.ejsto handle arrays in FormDataformdata-arrays.jsonto verify the fixUse Cases
This fix is essential for:
Testing
Backwards Compatibility
This change is fully backwards compatible: