Add blob support #1524
Add blob support #1524
Conversation
|
WalkthroughThe changes introduce support for Changes
Assessment against linked issues
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/core/src/v3/utils/ioSerialization.tsOops! Something went wrong! :( ESLint: 8.45.0 ESLint couldn't find the config "custom" to extend from. Please check that the name of the config is correct. The config "custom" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
tests/e2e/ioSerialization.test.ts (1)
4-13: Consider adding more test cases for Blob serialization.While the basic Blob test is good, consider adding these scenarios:
- Empty Blobs
- Blobs with different MIME types
- Large Blobs (near system limits)
- Error cases (e.g., corrupted data URLs)
packages/core/src/v3/utils/ioSerialization.ts (2)
201-210: Consider adding size limits for Blob handling.Large Blobs could cause memory issues when converting to data URLs. Consider adding size validation.
if (value instanceof Blob) { + const MAX_BLOB_SIZE = 50 * 1024 * 1024; // 50MB limit + if (value.size > MAX_BLOB_SIZE) { + throw new Error(`Blob size ${value.size} exceeds limit of ${MAX_BLOB_SIZE} bytes`); + } const reader = new FileReader(); return new Promise((resolve, reject) => {
236-243: Optimize Blob parsing performance.The current implementation uses a loop to copy bytes. Consider using more efficient methods.
-const byteString = atob(value.data.split(',')[1]); -const ab = new ArrayBuffer(byteString.length); -const ia = new Uint8Array(ab); -for (let i = 0; i < byteString.length; i++) { - ia[i] = byteString.charCodeAt(i); -} +const base64 = value.data.split(',')[1]; +const binary = atob(base64); +const bytes = Uint8Array.from(binary, char => char.charCodeAt(0));🧰 Tools
🪛 Biome (1.9.4)
[error] 236-236: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 237-237: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 238-238: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 239-239: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
packages/core/src/v3/utils/ioSerialization.ts(1 hunks)tests/e2e/ioSerialization.test.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/core/src/v3/utils/ioSerialization.ts
[error] 231-231: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 236-236: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 237-237: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 238-238: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 239-239: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 192-192: Shouldn't redeclare 'stringifyIO'. Consider to delete it or rename it.
'stringifyIO' is defined here:
(lint/suspicious/noRedeclare)
[error] 222-222: Shouldn't redeclare 'parsePacket'. Consider to delete it or rename it.
'parsePacket' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (2)
tests/e2e/ioSerialization.test.ts (1)
15-31: LGTM! Comprehensive tests for JSON and string handling.
The test cases for JSON objects and strings are well-structured and cover the basic scenarios.
packages/core/src/v3/utils/ioSerialization.ts (1)
235-244: Verify Blob type handling in the codebase.
Need to ensure that Blob handling is consistently implemented across the codebase.
✅ Verification successful
Let me gather more context about the implementation details by checking the test file and the full implementation.
Blob handling is correctly and consistently implemented
The implementation of Blob serialization and deserialization is well-tested and properly handled:
- Serialization converts Blobs to data URLs using FileReader
- Deserialization correctly reconstructs Blobs from data URLs preserving MIME types
- Test coverage confirms the roundtrip functionality
- The implementation is isolated to the IO serialization module and doesn't require changes elsewhere
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for Blob usage in the codebase
echo "Searching for Blob usage patterns..."
rg -l "instanceof Blob"
# Search for data URL handling
echo "Searching for data URL handling..."
rg "data:.*base64"
# Search for potential Blob consumers
echo "Searching for potential Blob consumers..."
ast-grep --pattern 'new Blob($$$)'
Length of output: 625
Script:
#!/bin/bash
# Check the test implementation
echo "Checking test implementation..."
cat tests/e2e/ioSerialization.test.ts
# Check the full serialization implementation
echo "Checking serialization implementation..."
cat packages/core/src/v3/utils/ioSerialization.ts
Length of output: 13881
🧰 Tools
🪛 Biome (1.9.4)
[error] 236-236: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 237-237: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 238-238: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 239-239: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
| // Add Blob support in the stringifyIO function | ||
| export async function stringifyIO(value: any): Promise<IOPacket> { | ||
| if (value === undefined) { | ||
| return { dataType: "application/json" }; | ||
| } | ||
|
|
||
| if (typeof value === "string") { | ||
| return { data: value, dataType: "text/plain" }; | ||
| } | ||
|
|
||
| if (value instanceof Blob) { | ||
| const reader = new FileReader(); | ||
| return new Promise((resolve, reject) => { | ||
| reader.onloadend = () => { | ||
| resolve({ data: reader.result as string, dataType: "application/blob" }); | ||
| }; | ||
| reader.onerror = reject; | ||
| reader.readAsDataURL(value); | ||
| }); | ||
| } | ||
|
|
||
| try { | ||
| const { stringify } = await loadSuperJSON(); | ||
| const data = stringify(value); | ||
| return { data, dataType: "application/super+json" }; | ||
| } catch { | ||
| return { data: value, dataType: "application/json" }; | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix function redeclaration and add error handling.
The stringifyIO function is redeclared, which will cause runtime errors. Additionally, the Blob handling needs better error handling.
Apply this diff to fix the issues:
-// Add Blob support in the stringifyIO function
-export async function stringifyIO(value: any): Promise<IOPacket> {
+// Update existing stringifyIO function
-if (value === undefined) {
- return { dataType: "application/json" };
-}
-
-if (typeof value === "string") {
- return { data: value, dataType: "text/plain" };
-}
-
+// Add Blob support
if (value instanceof Blob) {
const reader = new FileReader();
return new Promise((resolve, reject) => {
reader.onloadend = () => {
+ if (!reader.result) {
+ reject(new Error('Failed to read Blob'));
+ return;
+ }
resolve({ data: reader.result as string, dataType: "application/blob" });
};
reader.onerror = reject;
reader.readAsDataURL(value);
});
}
-try {
- const { stringify } = await loadSuperJSON();
- const data = stringify(value);
- return { data, dataType: "application/super+json" };
-} catch {
- return { data: value, dataType: "application/json" };
-}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 192-192: Shouldn't redeclare 'stringifyIO'. Consider to delete it or rename it.
'stringifyIO' is defined here:
(lint/suspicious/noRedeclare)
| // Add Blob support in the parsePacket function | ||
| export async function parsePacket(value: IOPacket): Promise<any> { | ||
| if (!value.data) { | ||
| return undefined; | ||
| } | ||
|
|
||
| switch (value.dataType) { | ||
| case "application/json": | ||
| return JSON.parse(value.data); | ||
| case "application/super+json": | ||
| const { parse } = await loadSuperJSON(); | ||
| return parse(value.data); | ||
| case "text/plain": | ||
| return value.data; | ||
| case "application/blob": | ||
| const byteString = atob(value.data.split(',')[1]); | ||
| const mimeString = value.data.split(',')[0].split(':')[1].split(';')[0]; | ||
| const ab = new ArrayBuffer(byteString.length); | ||
| const ia = new Uint8Array(ab); | ||
| for (let i = 0; i < byteString.length; i++) { | ||
| ia[i] = byteString.charCodeAt(i); | ||
| } | ||
| return new Blob([ab], { type: mimeString }); | ||
| case "application/store": | ||
| throw new Error(`Cannot parse an application/store packet (${value.data}). Needs to be imported first.`); | ||
| default: | ||
| return value.data; | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix switch case declarations and add validation.
The switch case declarations need block scoping, and the Blob parsing needs input validation.
Apply this diff to fix the issues:
-// Add Blob support in the parsePacket function
-export async function parsePacket(value: IOPacket): Promise<any> {
case "application/blob": {
+ if (!value.data || !value.data.includes(',')) {
+ throw new Error('Invalid data URL format');
+ }
const byteString = atob(value.data.split(',')[1]);
const mimeString = value.data.split(',')[0].split(':')[1].split(';')[0];
+ if (!mimeString) {
+ throw new Error('Invalid MIME type in data URL');
+ }
const ab = new ArrayBuffer(byteString.length);
const ia = new Uint8Array(ab);
for (let i = 0; i < byteString.length; i++) {
ia[i] = byteString.charCodeAt(i);
}
return new Blob([ab], { type: mimeString });
+ break;
}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 231-231: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 236-236: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 237-237: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 238-238: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 239-239: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 222-222: Shouldn't redeclare 'parsePacket'. Consider to delete it or rename it.
'parsePacket' is defined here:
(lint/suspicious/noRedeclare)
Closes #1523
✅ Checklist
Serialization and Deserialization
Blob Support
The
ioSerialization.tsfile now supports serialization and deserialization of Blob objects.Example Usage