feat: Add support for file upload as binary data via Buffer, Readable, ReadableStream#2925
Merged
mtrezza merged 17 commits intoparse-community:alphafrom Feb 20, 2026
Merged
Conversation
|
🚀 Thanks for opening this pull request! |
Contributor
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
📝 WalkthroughWalkthroughAdds Buffer and Stream file sources, an optional FileController.saveBinary API, DefaultController binary upload handling, ReadableStream support in RESTController, type updates, and extensive unit/integration tests for buffer/stream save/getData and auth/header behaviors. Changes
Sequence DiagramsequenceDiagram
participant Client as ParseFile Client
participant FileCtrl as FileController / DefaultController
participant REST as RESTController
participant Server as Parse Server
Client->>FileCtrl: save(name, source{buffer|stream}, options)
FileCtrl->>REST: prepare upload (decide saveBinary vs saveBase64)
alt source.format == 'stream' and has metadata/tags
FileCtrl-->>Client: Error("cannot save stream with metadata/tags")
else source in {'buffer','stream'} and no metadata/tags
alt FileCtrl.saveBinary exists
REST->>Server: HTTP POST (raw body, Content-Type, auth headers, duplex='half' for ReadableStream)
Server-->>REST: { name, url }
else source == 'buffer'
REST->>Server: HTTP POST (JSON with base64, metadata)
Server-->>REST: { name, url }
else
FileCtrl-->>Client: Error("saveBinary required for stream uploads")
end
end
REST-->>FileCtrl: { name, url }
FileCtrl-->>Client: return { name, url }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (6)
src/__tests__/ParseFile-test.js (2)
601-611: Consider usingexpect.assertions(1)for expected-throw tests.Multiple tests use the
expect(true).toBe(false)anti-pattern to fail if no error is thrown (lines 607, 1048, 1063, 1105, 1158). If the async function resolves unexpectedly, the test could pass silently if thecatchblock has a bug. Usingexpect.assertions(n)orawait expect(...).rejects.toThrow(...)is more idiomatic Jest and provides a clearer failure message.♻️ Example using rejects.toThrow
it('getData from stream-backed unsaved file throws', async () => { const { Readable } = require('stream'); const stream = new Readable({ read() { this.push(null); } }); const file = new ParseFile('parse.txt', stream, 'text/plain'); - try { - await file.getData(); - expect(true).toBe(false); - } catch (e) { - expect(e.message).toBe('Cannot retrieve data for unsaved ParseFile.'); - } + await expect(file.getData()).rejects.toThrow('Cannot retrieve data for unsaved ParseFile.'); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/ParseFile-test.js` around lines 601 - 611, The test for ParseFile.getData uses the anti-pattern expect(true).toBe(false) to assert a throw; update the test (and the other similar tests referencing ParseFile usage) to use Jest's idiomatic assertions by either calling expect.assertions(1) at the top of the test or converting the try/catch to await expect(file.getData()).rejects.toThrow('Cannot retrieve data for unsaved ParseFile.'); target the test that constructs new ParseFile('parse.txt', stream, 'text/plain') and any other occurrences noted (the tests around the getData calls) and replace the fail-on-success pattern with one of these approaches.
886-966: Missing test: saving a Web ReadableStream (non-Node.js stream) viasaveBinary.The stream save test at line 932 uses a Node.js
Readable, which exercises theReadable.toWeb()conversion path (line 626-627 of ParseFile.ts). There's no test for passing a WebReadableStreamdirectly tosaveBinary, which would exercise theelsebranch at line 634 of ParseFile.ts where the stream is used as-is.Consider adding a test that creates a
new ReadableStream(...)(fromstream/web) and verifies it's passed directly toajaxwithout conversion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/ParseFile-test.js` around lines 886 - 966, Add a test that covers passing a Web ReadableStream directly into ParseFile.save/saveBinary so the code path that uses the stream as-is is exercised: create a Web ReadableStream (imported from 'stream/web' or global ReadableStream), pass it into new ParseFile('parse.txt', webStream, 'text/plain'), set up CoreManager RESTController.ajax mock as in other tests, call file.save(), and assert ajax was called with the request body equal to that Web ReadableStream (or at least that ajax received the same object reference) and headers include the correct Content-Type and X-Parse-Application-ID; this will exercise the else branch in ParseFile.saveBinary that does not call Readable.toWeb().src/ParseFile.ts (3)
574-648:saveBinarybypassesRESTController.request()— duplicated auth/header logic.
saveBinarymanually constructs auth headers (Application ID, JS Key, Master Key, Session Token) and callsajax()directly, duplicating logic fromRESTController.request(). This meanssaveBinarymisses:
- Installation ID header
FORCE_REVOCABLE_SESSIONflag- Custom
REQUEST_HEADERS(thoughajaxadds these internally at line 149 of RESTController.ts)IDEMPOTENCYrequest IDThe existing
deleteFilemethod follows the sameajax-direct pattern, so this is at least consistent with the codebase. However, it's worth considering whether the binary upload should also include installation ID and idempotency headers for parity withsaveBase64(which goes throughrequest()).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ParseFile.ts` around lines 574 - 648, saveBinary constructs auth headers and calls CoreManager.getRESTController().ajax() directly, duplicating logic and omitting headers added by RESTController.request() (e.g., installation ID, FORCE_REVOCABLE_SESSION, REQUEST_HEADERS, IDEMPOTENCY); update saveBinary to delegate to RESTController.request() like saveBase64 (or, if ajax must be used, replicate all header injection performed by RESTController.request()), ensuring you include installation ID, FORCE_REVOCABLE_SESSION flag, any CoreManager REQUEST_HEADERS, and an idempotency request ID; reference the saveBinary function and CoreManager.getRESTController().request()/ajax(), and mirror how deleteFile or saveBase64 handle auth/session headers to keep behavior consistent.
619-637:require('stream')will be included in browser bundles.Line 625 uses
require('stream')inside a conditional block. While the runtime guard (typeof stream.pipe === 'function') prevents execution in browsers, bundlers (Webpack, Rollup, etc.) will still include thestreammodule in the output unless it's externalized. This inflates bundle size for browser consumers.Consider using a dynamic
import()or documenting that bundler configs should externalizestreamfor browser targets. This is consistent with pre-existing patterns in the SDK but worth noting for bundle-conscious users.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ParseFile.ts` around lines 619 - 637, The current use of require('stream') inside the guarded block causes bundlers to include the Node 'stream' module; replace the synchronous require with a dynamic import to keep the module out of browser bundles: change the code in ParseFile (the branch handling source.format === 'stream') to use await import('stream') (or import('stream').then(...)) to obtain Readable and then check typeof Readable.toWeb === 'function' before calling Readable.toWeb(stream); update the containing function to be async (or handle the returned promise) so the import can be awaited, and keep the existing fallback/error behavior if Readable.toWeb is unavailable.
639-647: URL construction duplicatesdeleteFile— consider extracting a helper.Lines 639-643 repeat the same
SERVER_URL+ trailing slash +'files/' + namepattern found indeleteFile(lines 703-707). A small helper likegetFileUrl(name)would reduce duplication.♻️ Suggested helper extraction
+function getFileUrl(name: string): string { + let url = CoreManager.get('SERVER_URL'); + if (url[url.length - 1] !== '/') { + url += '/'; + } + return url + 'files/' + name; +} + const DefaultController = { // ... saveBinary: async function (...) { // ... - let url = CoreManager.get('SERVER_URL'); - if (url[url.length - 1] !== '/') { - url += '/'; - } - url += 'files/' + name; + const url = getFileUrl(name); // ... }, // ... deleteFile: function (...) { // ... - let url = CoreManager.get('SERVER_URL'); - if (url[url.length - 1] !== '/') { - url += '/'; - } - url += 'files/' + name; + const url = getFileUrl(name); // ... }, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ParseFile.ts` around lines 639 - 647, The URL construction for file endpoints is duplicated in this file (the block building url from CoreManager.get('SERVER_URL') and appending 'files/' + name appears in this method and in deleteFile); extract a small helper function getFileUrl(name) inside ParseFile.ts that reads CoreManager.get('SERVER_URL'), ensures a trailing '/', and returns serverUrl + 'files/' + name, then replace the inline construction in both the current method and deleteFile with calls to getFileUrl(name) to remove duplication and keep behavior identical.types/ParseFile.d.ts (1)
25-32: Consider using more specific types forbufferandstreaminstead ofany.Using
anyforbufferandstreamloses type safety. Even in a cross-environment declaration file, you could narrow these toBuffer(or{ toString(encoding: string): string }) andReadableStream | NodeJS.ReadableStreamrespectively, behind conditional/union types. This would give consumers better autocompletion and catch misuse at compile time.That said, this is a pragmatic choice given the cross-environment constraints.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@types/ParseFile.d.ts` around lines 25 - 32, The union branch for ParseFile that uses format:'buffer' and format:'stream' currently types properties buffer and stream as any; change them to more specific types to restore type safety: for the buffer branch replace buffer:any with a narrower type such as Buffer | { toString(encoding?: string): string } (or a union that covers both browser and Node), and for the stream branch replace stream:any with ReadableStream | NodeJS.ReadableStream (or a union that covers both environments); update the ParseFile union branches (format: 'buffer' / format: 'stream') accordingly so consumers get proper autocompletion and compile-time checks while keeping cross-environment compatibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/__tests__/ParseFile-test.js`:
- Around line 601-611: The test for ParseFile.getData uses the anti-pattern
expect(true).toBe(false) to assert a throw; update the test (and the other
similar tests referencing ParseFile usage) to use Jest's idiomatic assertions by
either calling expect.assertions(1) at the top of the test or converting the
try/catch to await expect(file.getData()).rejects.toThrow('Cannot retrieve data
for unsaved ParseFile.'); target the test that constructs new
ParseFile('parse.txt', stream, 'text/plain') and any other occurrences noted
(the tests around the getData calls) and replace the fail-on-success pattern
with one of these approaches.
- Around line 886-966: Add a test that covers passing a Web ReadableStream
directly into ParseFile.save/saveBinary so the code path that uses the stream
as-is is exercised: create a Web ReadableStream (imported from 'stream/web' or
global ReadableStream), pass it into new ParseFile('parse.txt', webStream,
'text/plain'), set up CoreManager RESTController.ajax mock as in other tests,
call file.save(), and assert ajax was called with the request body equal to that
Web ReadableStream (or at least that ajax received the same object reference)
and headers include the correct Content-Type and X-Parse-Application-ID; this
will exercise the else branch in ParseFile.saveBinary that does not call
Readable.toWeb().
In `@src/ParseFile.ts`:
- Around line 574-648: saveBinary constructs auth headers and calls
CoreManager.getRESTController().ajax() directly, duplicating logic and omitting
headers added by RESTController.request() (e.g., installation ID,
FORCE_REVOCABLE_SESSION, REQUEST_HEADERS, IDEMPOTENCY); update saveBinary to
delegate to RESTController.request() like saveBase64 (or, if ajax must be used,
replicate all header injection performed by RESTController.request()), ensuring
you include installation ID, FORCE_REVOCABLE_SESSION flag, any CoreManager
REQUEST_HEADERS, and an idempotency request ID; reference the saveBinary
function and CoreManager.getRESTController().request()/ajax(), and mirror how
deleteFile or saveBase64 handle auth/session headers to keep behavior
consistent.
- Around line 619-637: The current use of require('stream') inside the guarded
block causes bundlers to include the Node 'stream' module; replace the
synchronous require with a dynamic import to keep the module out of browser
bundles: change the code in ParseFile (the branch handling source.format ===
'stream') to use await import('stream') (or import('stream').then(...)) to
obtain Readable and then check typeof Readable.toWeb === 'function' before
calling Readable.toWeb(stream); update the containing function to be async (or
handle the returned promise) so the import can be awaited, and keep the existing
fallback/error behavior if Readable.toWeb is unavailable.
- Around line 639-647: The URL construction for file endpoints is duplicated in
this file (the block building url from CoreManager.get('SERVER_URL') and
appending 'files/' + name appears in this method and in deleteFile); extract a
small helper function getFileUrl(name) inside ParseFile.ts that reads
CoreManager.get('SERVER_URL'), ensures a trailing '/', and returns serverUrl +
'files/' + name, then replace the inline construction in both the current method
and deleteFile with calls to getFileUrl(name) to remove duplication and keep
behavior identical.
In `@types/ParseFile.d.ts`:
- Around line 25-32: The union branch for ParseFile that uses format:'buffer'
and format:'stream' currently types properties buffer and stream as any; change
them to more specific types to restore type safety: for the buffer branch
replace buffer:any with a narrower type such as Buffer | { toString(encoding?:
string): string } (or a union that covers both browser and Node), and for the
stream branch replace stream:any with ReadableStream | NodeJS.ReadableStream (or
a union that covers both environments); update the ParseFile union branches
(format: 'buffer' / format: 'stream') accordingly so consumers get proper
autocompletion and compile-time checks while keeping cross-environment
compatibility.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #2925 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 64 64
Lines 6235 6318 +83
Branches 1493 1529 +36
=========================================
+ Hits 6235 6318 +83 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/__tests__/ParseFile-test.js (2)
605-615: Optional: guard error-path tests withexpect.assertions(n).The
expect(true).toBe(false)sentinel is used withoutexpect.assertions(1), so ifgetData()resolves instead of rejects (e.g., after a future refactor), the test passes vacuously — only the unreachable sentinel fires, while the catch block (and the message check) is silently skipped.🛠️ Proposed fix
it('getData from stream-backed unsaved file throws', async () => { const { Readable } = require('stream'); const stream = new Readable({ read() { this.push(null); } }); const file = new ParseFile('parse.txt', stream, 'text/plain'); + expect.assertions(1); try { await file.getData(); - expect(true).toBe(false); } catch (e) { expect(e.message).toBe('Cannot retrieve data for unsaved ParseFile.'); } });The same pattern appears in multiple new error-path tests (lines 1065–1070, 1107–1113, 1160–1166). Applying
expect.assertions(1)consistently there too would remove the sentinel lines and make failures unambiguous.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/ParseFile-test.js` around lines 605 - 615, The error-path test for ParseFile.getData uses a sentinel (expect(true).toBe(false)) which can mask a missing rejection; replace the sentinel by adding expect.assertions(1) at the start of the test and keep the existing try/catch assertion that checks e.message === 'Cannot retrieve data for unsaved ParseFile.'; apply the same change (add expect.assertions(1) and remove sentinels) to the other error-path tests that use the sentinel (the tests around getData/error checks at the other indicated locations).
993-1021:UserControlleroverride not restored — minor test isolation leak.
CoreManager.set('UserController', { currentUserAsync: … })at line 994 is never reverted. Subsequent tests within theFileControllerdescribe block will executesaveBinarywith an activecurrentUserAsyncthat injects'currentUserToken'as a session token header, which is unintended side-state. This is consistent with the existing pattern at lines 667–705, but worth noting as each test should be self-contained.🛠️ Proposed fix
it('saveBinary includes session token from currentUserAsync', async () => { + const originalUserController = CoreManager.get('UserController'); CoreManager.set('UserController', { currentUserAsync() { return Promise.resolve({ getSessionToken() { return 'currentUserToken'; } }); }, }); ... expect(ajax).toHaveBeenCalledWith(...); + CoreManager.set('UserController', originalUserController); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/ParseFile-test.js` around lines 993 - 1021, The test overrides CoreManager's UserController via CoreManager.set('UserController', { currentUserAsync() { ... } }) and never restores it, leaking state; wrap the override so you save the original controller (const original = CoreManager.get('UserController')) before calling CoreManager.set and restore it after the async test (in a finally block or using afterEach) by calling CoreManager.set('UserController', original) so subsequent tests won't see the stubbed currentUserAsync/session token.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/__tests__/ParseFile-test.js`:
- Around line 890-919: The buffer-backed test "saves buffer-backed files via
saveBinary" leaves CoreManager state dirty by setting
CoreManager.set('JAVASCRIPT_KEY','testJsKey') without cleanup; update that test
to restore/clear the key after the test (e.g., call
CoreManager.set('JAVASCRIPT_KEY', null) or use an afterEach) so it doesn't leak
into other tests, and in the master-key test that asserts removal of the header,
explicitly set CoreManager.set('JAVASCRIPT_KEY','someKey') at the start of that
test so the assertion on headers
(expect(...).not.toHaveProperty('X-Parse-JavaScript-Key')) actually verifies
deletion rather than passing vacuously.
- Around line 430-448: The Web ReadableStream test for ParseFile is missing the
assertion that _data is undefined; update the test within the ParseFile 'can
create files with a Web ReadableStream' case to include the same assertion as
the Node Readable stream test by adding an assertion that file._data is
undefined so both stream code paths behave identically (refer to ParseFile usage
and the test block that constructs the Web ReadableStream and inspects
file._source).
---
Nitpick comments:
In `@src/__tests__/ParseFile-test.js`:
- Around line 605-615: The error-path test for ParseFile.getData uses a sentinel
(expect(true).toBe(false)) which can mask a missing rejection; replace the
sentinel by adding expect.assertions(1) at the start of the test and keep the
existing try/catch assertion that checks e.message === 'Cannot retrieve data for
unsaved ParseFile.'; apply the same change (add expect.assertions(1) and remove
sentinels) to the other error-path tests that use the sentinel (the tests around
getData/error checks at the other indicated locations).
- Around line 993-1021: The test overrides CoreManager's UserController via
CoreManager.set('UserController', { currentUserAsync() { ... } }) and never
restores it, leaking state; wrap the override so you save the original
controller (const original = CoreManager.get('UserController')) before calling
CoreManager.set and restore it after the async test (in a finally block or using
afterEach) by calling CoreManager.set('UserController', original) so subsequent
tests won't see the stubbed currentUserAsync/session token.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ParseFile.ts (1)
17-17:⚠️ Potential issue | 🟠 Major
FileDatatype missingBufferandReadableStream— TypeScript callers cannot pass these types without casting.The
FileDataunion at line 17 isnumber[] | Base64 | Blob | Uri, and this definition is repeated intypes/ParseFile.d.ts. NeitherBuffernorReadableStreamis included. The constructor parameter at line 114 usesdata?: FileData, so TypeScript will reject these types at compile time, even though the runtime detection logic at lines 122–144 handles them correctly via duck-typing and instanceof checks.Additionally,
FileSourcein the declaration file includesformat: 'buffer'andformat: 'stream'variants, confirming the internal support for these types, but the public input typeFileDatawas not updated accordingly.Suggested fix
Update the
FileDatatype in bothsrc/ParseFile.ts(line 17) andtypes/ParseFile.d.ts(line 8):-type FileData = number[] | Base64 | Blob | Uri; +type FileData = number[] | Base64 | Blob | Uri | Buffer | ReadableStream;Note: For stricter typing, consider using
ReadableStream<Uint8Array>for the Web API variant and a Node.js-specific type for the Node stream.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ParseFile.ts` at line 17, The FileData union in ParseFile.ts (type FileData) and its duplicate in types/ParseFile.d.ts is missing Buffer and readable-stream types, causing TypeScript callers to need casts even though ParseFile constructor (data?: FileData) and runtime checks (duck-typing/instanceof in the constructor logic) support them; update the FileData union to include Node Buffer and stream types (e.g., Buffer and the appropriate Node Readable type) and also include the Web ReadableStream variant (e.g., ReadableStream<Uint8Array>) in both the source file and the declaration file so TypeScript callers can pass Buffer and streams without casting, keeping the constructor signature (data?: FileData) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ParseFile.ts`:
- Around line 311-319: The save() implementation throws synchronously when
this._source.format === 'stream' (two places checking hasMetadataOrTags and
controller.saveBinary); change those synchronous throws to return rejected
Promises so callers using promise .catch() will receive the error—either convert
save() to async and keep throws, or replace each throw new Error(...) with
returning Promise.reject(new Error(...)); update the checks in the save()
function where the stream-format errors are raised (the two blocks referencing
hasMetadataOrTags and controller.saveBinary) accordingly.
- Line 329: The property _data on the ParseFile class is typed as string |
undefined but is being assigned null; change those assignments to undefined to
match the declared type—update the assignment where you currently set this._data
= null and the similar assignment in the saveFile path to this._data =
undefined; ensure no other code relies on null checks (convert any `=== null`
checks to `=== undefined` or use == null if intentionally handling both).
- Around line 624-651: When the file upload code converts a Node Readable to a
Web ReadableStream in ParseFile (body = NodeReadable.toWeb(stream)), the
RESTController fetch call must pass duplex: 'half' for streaming request bodies;
update the fetch invocation in RESTController.ajax (the method that uses
options.requestTask/AbortController and performs the fetch) to detect if the
request body is a ReadableStream (e.g., using body instanceof
globalThis.ReadableStream or checking typeof body.getReader === 'function') and,
if so, include duplex: 'half' in the fetch init options before calling fetch.
---
Outside diff comments:
In `@src/ParseFile.ts`:
- Line 17: The FileData union in ParseFile.ts (type FileData) and its duplicate
in types/ParseFile.d.ts is missing Buffer and readable-stream types, causing
TypeScript callers to need casts even though ParseFile constructor (data?:
FileData) and runtime checks (duck-typing/instanceof in the constructor logic)
support them; update the FileData union to include Node Buffer and stream types
(e.g., Buffer and the appropriate Node Readable type) and also include the Web
ReadableStream variant (e.g., ReadableStream<Uint8Array>) in both the source
file and the declaration file so TypeScript callers can pass Buffer and streams
without casting, keeping the constructor signature (data?: FileData) unchanged.
mtrezza
changed the title
Buffer, Readable, ReadableStreamBuffer, Readable, ReadableStream
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ParseFile.ts (1)
17-17:⚠️ Potential issue | 🟠 Major
FileDatatype doesn't includeBufferorReadableStream— TypeScript callers will get type errors.The constructor now accepts
BufferandReadableStreamat runtime, butFileData(used as thedataparameter type) is still:type FileData = number[] | Base64 | Blob | Uri;A TypeScript caller doing
new ParseFile('name', Buffer.from([...]))gets:
Argument of type 'Buffer' is not assignable to parameter of type 'FileData | undefined'.The same problem applies to
NodeJS.ReadableandReadableStream. The same gap exists intypes/ParseFile.d.tsLine 8.🐛 Proposed fix
-type FileData = number[] | Base64 | Blob | Uri; +type FileData = number[] | Base64 | Blob | Uri | Buffer | ReadableStream | NodeJS.ReadableStream;Apply the same change to
types/ParseFile.d.tsLine 8.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ParseFile.ts` at line 17, The FileData union type is missing Buffer and Readable/ReadableStream variants so TypeScript callers get type errors when passing Buffer or streams to the ParseFile constructor; update the FileData type definition (the type alias named FileData in src/ParseFile.ts and the parallel declaration in types/ParseFile.d.ts) to include Buffer and NodeJS.Readable | ReadableStream (or the appropriate stream type used elsewhere) in the union so the constructor's data parameter accepts Buffer and streams at compile time as it does at runtime.
🧹 Nitpick comments (1)
types/ParseFile.d.ts (1)
25-33:buffer: anyandstream: anycould use stricter types.Using
anysacrifices type safety for these fields. Consider narrower types such asBuffer | ArrayBuffer | Uint8ArrayforbufferandReadableStream | NodeJS.ReadableStream(or an intersection) forstream, consistent with the actual runtime checks in the constructor.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@types/ParseFile.d.ts` around lines 25 - 33, The union variant with format: 'buffer' and the variant with format: 'stream' use overly broad any types for the buffer and stream fields; change the buffer field in the { format: 'buffer' } variant from any to a stricter union such as Buffer | ArrayBuffer | Uint8Array and change the stream field in the { format: 'stream' } variant from any to a stream type such as ReadableStream | NodeJS.ReadableStream (or an appropriate intersection) so the declaration matches the runtime constructor checks and improves type safety for the ParseFile type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/__tests__/ParseFile-test.js`:
- Around line 995-1023: The UserController mock set via
CoreManager.set('UserController', { currentUserAsync() { ... } }) in the
"saveBinary includes session token from currentUserAsync" test leaks into the
next master-key test; save the original controller before overriding (e.g.,
const originalUserController = CoreManager.get('UserController')) and restore it
after the test (in a finally block or afterEach) so ParseFile.save / saveBinary
tests do not inherit the mocked currentUserAsync; ensure the restore uses
CoreManager.set('UserController', originalUserController) and that the cleanup
runs even if the test fails.
---
Outside diff comments:
In `@src/ParseFile.ts`:
- Line 17: The FileData union type is missing Buffer and Readable/ReadableStream
variants so TypeScript callers get type errors when passing Buffer or streams to
the ParseFile constructor; update the FileData type definition (the type alias
named FileData in src/ParseFile.ts and the parallel declaration in
types/ParseFile.d.ts) to include Buffer and NodeJS.Readable | ReadableStream (or
the appropriate stream type used elsewhere) in the union so the constructor's
data parameter accepts Buffer and streams at compile time as it does at runtime.
---
Duplicate comments:
In `@src/ParseFile.ts`:
- Around line 316-324: The synchronous throws inside ParseFile.save are breaking
the promise contract; replace both throw statements (the checks using
this._source.format === 'stream' && hasMetadataOrTags and this._source.format
=== 'stream' && !controller.saveBinary) with returning rejected promises (e.g.,
return Promise.reject(new Error(...))) so errors are delivered through the
promise chain instead of thrown synchronously; update the save() method in the
ParseFile class to return Promise.reject(...) for those two error cases.
- Line 334: The assignment this._data = null violates the declared type
(this._data: string | undefined) in the ParseFile class; change the assignment
to this._data = undefined so it matches the type used elsewhere in the class
(look for other uses of _data and methods like parse()/reset() that rely on
undefined) and update any related null checks to check for undefined if
necessary.
- Around line 629-656: The streaming upload path currently converts Node streams
via NodeReadable.toWeb and passes the resulting ReadableStream into
RESTController.ajax without setting duplex:'half' and without handling retries;
update RESTController.ajax so that when the request body is a ReadableStream
(detect via isReadableStream or instanceof check) you (1) include
RequestInit.duplex = 'half' when using fetch with a Web ReadableStream body (so
Node.js/undici accepts the body) and (2) disable automatic retry logic for that
request body (i.e., do not re-run the captured dispatch closure that would reuse
a consumed ReadableStream) so that 5xx responses propagate instead of causing
"Body is unusable" errors; ensure calls originating from ParseFile (where
NodeReadable.toWeb is used) will therefore send duplex:'half' and skip retries
for stream bodies.
---
Nitpick comments:
In `@types/ParseFile.d.ts`:
- Around line 25-33: The union variant with format: 'buffer' and the variant
with format: 'stream' use overly broad any types for the buffer and stream
fields; change the buffer field in the { format: 'buffer' } variant from any to
a stricter union such as Buffer | ArrayBuffer | Uint8Array and change the stream
field in the { format: 'stream' } variant from any to a stream type such as
ReadableStream | NodeJS.ReadableStream (or an appropriate intersection) so the
declaration matches the runtime constructor checks and improves type safety for
the ParseFile type.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/RESTController.ts`:
- Around line 160-164: The retry logic is reusing the same ReadableStream body
which becomes locked after the first fetch and causes a misleading TypeError on
retry; update dispatch (which uses fetchOptions and fetchOptions.body) to detect
when fetchOptions.body is a ReadableStream (typeof ReadableStream !==
'undefined' && fetchOptions.body instanceof ReadableStream) and in that case
disable/skip the retry path on 5XX / status-0 responses so the original server
error/response is propagated instead of attempting to re-invoke dispatch with
the same stream; ensure the fetch still sets fetchOptions.duplex = 'half' for
stream bodies but do not enqueue any retry attempts when the body is a
ReadableStream.
Member
Author
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ParseFile.ts`:
- Around line 127-149: The FileData union type is missing binary and stream
variants that the ParseFile constructor handles (Uint8Array, Buffer,
NodeJS.ReadableStream, ReadableStream); update the FileData type definition used
by ParseFile to include Uint8Array, Buffer, NodeJS.ReadableStream, and
ReadableStream so TypeScript callers match runtime behavior. Modify both the
runtime source type (in src/ParseFile.ts where the ParseFile constructor and
this._source handling exist) and the declaration file (types/ParseFile.d.ts
where FileData is declared) to add these union members, keeping existing members
(number[] | Base64 | Blob | Uri) intact. Ensure imports/ambient types for
NodeJS.ReadableStream and global ReadableStream are referenced or declared so
the types compile in both Node and browser environments.
Comment on lines
+127
to
+149
| if (typeof Buffer !== 'undefined' && Buffer.isBuffer(data)) { | ||
| this._source = { | ||
| format: 'buffer', | ||
| buffer: data, | ||
| type: specifiedType, | ||
| }; | ||
| } else if ( | ||
| data !== null && | ||
| typeof data === 'object' && | ||
| typeof (data as any).pipe === 'function' && | ||
| typeof (data as any).read === 'function' | ||
| ) { | ||
| this._source = { | ||
| format: 'stream', | ||
| stream: data, | ||
| type: specifiedType, | ||
| }; | ||
| } else if (typeof ReadableStream !== 'undefined' && data instanceof ReadableStream) { | ||
| this._source = { | ||
| format: 'stream', | ||
| stream: data, | ||
| type: specifiedType, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the FileData type definition in src/ParseFile.ts
rg "type FileData" src/ParseFile.ts -A 1
# Find the constructor signature in types/ParseFile.d.ts
fd -e d.ts "ParseFile" | head -5Repository: parse-community/Parse-SDK-JS
Length of output: 186
🏁 Script executed:
#!/bin/bash
# Show constructor and FileData context in types/ParseFile.d.ts
find . -name "ParseFile.d.ts" -type f | head -1Repository: parse-community/Parse-SDK-JS
Length of output: 94
🏁 Script executed:
#!/bin/bash
# Search for ParseFile.d.ts location and examine it
fd "ParseFile.d.ts" --type fRepository: parse-community/Parse-SDK-JS
Length of output: 92
🏁 Script executed:
#!/bin/bash
# Read the types/ParseFile.d.ts file to see constructor and FileData definition
cat types/ParseFile.d.ts | head -100Repository: parse-community/Parse-SDK-JS
Length of output: 3817
🏁 Script executed:
#!/bin/bash
# Check src/ParseFile.ts for FileData type definition context
rg "type FileData" src/ParseFile.ts -B 2 -A 2Repository: parse-community/Parse-SDK-JS
Length of output: 216
🏁 Script executed:
#!/bin/bash
# Verify constructor parameter handling in src/ParseFile.ts
sed -n '116,150p' src/ParseFile.tsRepository: parse-community/Parse-SDK-JS
Length of output: 1275
🏁 Script executed:
#!/bin/bash
# Check package.json for Node.js version requirements
cat package.json | grep -A 10 '"engines"'Repository: parse-community/Parse-SDK-JS
Length of output: 326
🏁 Script executed:
#!/bin/bash
# Check the save() method implementation at lines 311-353 for synchronous throws
sed -n '311,353p' src/ParseFile.tsRepository: parse-community/Parse-SDK-JS
Length of output: 1782
🏁 Script executed:
#!/bin/bash
# Check line 334 for _data = null pattern
sed -n '330,345p' src/ParseFile.tsRepository: parse-community/Parse-SDK-JS
Length of output: 712
🏁 Script executed:
#!/bin/bash
# Verify what types are actually accepted by checking the full constructor implementation
sed -n '116,200p' src/ParseFile.ts | grep -A 5 "constructor\|FileData"Repository: parse-community/Parse-SDK-JS
Length of output: 295
🏁 Script executed:
#!/bin/bash
# Check if there are overloaded constructor signatures or any type guards
rg "constructor.*FileData|Buffer|Readable" src/ParseFile.ts types/ParseFile.d.tsRepository: parse-community/Parse-SDK-JS
Length of output: 1737
🏁 Script executed:
#!/bin/bash
# Get more context around NodeReadable.toWeb usage to verify Node.js version requirement
rg "NodeReadable.toWeb" src/ParseFile.ts -B 3 -A 3Repository: parse-community/Parse-SDK-JS
Length of output: 325
🏁 Script executed:
#!/bin/bash
# Check the saveBinary implementation to see if NodeReadable.toWeb is guarded
sed -n '584,650p' src/ParseFile.tsRepository: parse-community/Parse-SDK-JS
Length of output: 2223
🏁 Script executed:
#!/bin/bash
# Verify if there's any Uint8Array handling that should be in FileData
rg "Uint8Array" src/ParseFile.ts types/ParseFile.d.ts -B 1 -A 1Repository: parse-community/Parse-SDK-JS
Length of output: 1206
Update FileData type to include Uint8Array, Buffer, and stream types.
The constructor accepts Buffer, Uint8Array, NodeJS.ReadableStream, and ReadableStream, but the FileData type definition (number[] | Base64 | Blob | Uri) excludes them. TypeScript users passing these types will get compilation errors, despite the documentation and runtime implementation supporting them.
Both src/ParseFile.ts and types/ParseFile.d.ts need the type updated:
📝 Proposed fix
-type FileData = number[] | Base64 | Blob | Uri;
+type FileData = number[] | Uint8Array | Base64 | Blob | Uri | Buffer | NodeJS.ReadableStream | ReadableStream;Apply the same update to both files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ParseFile.ts` around lines 127 - 149, The FileData union type is missing
binary and stream variants that the ParseFile constructor handles (Uint8Array,
Buffer, NodeJS.ReadableStream, ReadableStream); update the FileData type
definition used by ParseFile to include Uint8Array, Buffer,
NodeJS.ReadableStream, and ReadableStream so TypeScript callers match runtime
behavior. Modify both the runtime source type (in src/ParseFile.ts where the
ParseFile constructor and this._source handling exist) and the declaration file
(types/ParseFile.d.ts where FileData is declared) to add these union members,
keeping existing members (number[] | Base64 | Blob | Uri) intact. Ensure
imports/ambient types for NodeJS.ReadableStream and global ReadableStream are
referenced or declared so the types compile in both Node and browser
environments.
parseplatformorg
pushed a commit
that referenced
this pull request
Feb 20, 2026
# [8.2.0-alpha.1](8.1.1-alpha.6...8.2.0-alpha.1) (2026-02-20) ### Features * Add support for file upload as binary data via `Buffer`, `Readable`, `ReadableStream` ([#2925](#2925)) ([e42caf6](e42caf6))
Contributor
|
🎉 This change has been released in version 8.2.0-alpha.1 |
parseplatformorg
pushed a commit
that referenced
this pull request
Feb 20, 2026
# [8.2.0](8.1.0...8.2.0) (2026-02-20) ### Bug Fixes * `Parse.Object.createWithoutData` doesn't preserve object subclass ([#2907](#2907)) ([01dc94d](01dc94d)) * `Parse.Query.and/or/nor` loosing custom class types ([#2903](#2903)) ([89fdb07](89fdb07)) * `Parse.serverURL` not accessible via global `Parse` scope ([#2917](#2917)) ([4e78681](4e78681)) * Cloud trigger type errors for void returns and subclass constructors ([#2904](#2904)) ([de9f56d](de9f56d)) * Missing or incorrect type exports ([#2909](#2909)) ([3caa4ec](3caa4ec)) * Type error in `Parse.Query.equalTo` when matching optional array ([#2901](#2901)) ([8c96da9](8c96da9)) ### Features * Add request header `X-Parse-Upload-Mode` to identify file upload as binary data via `Buffer`, `Readable`, `ReadableStream` ([#2927](#2927)) ([a66bb06](a66bb06)) * Add support for file upload as binary data via `Buffer`, `Readable`, `ReadableStream` ([#2925](#2925)) ([e42caf6](e42caf6))
Contributor
|
🎉 This change has been released in version 8.2.0 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Issue
Add support for file upload via
Buffer,Readable,ReadableStream.Summary by CodeRabbit
New Features
Behavior Changes
Documentation / Types
Tests