Skip to content
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

Add blob support #1524

Closed
wants to merge 2 commits into from
Closed
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
58 changes: 58 additions & 0 deletions packages/core/src/v3/utils/ioSerialization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,65 @@ export async function conditionallyImportPacket(
return result ?? packet;
}
}
// 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" };
}
}
Comment on lines +191 to +219
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

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;
}
}
Comment on lines +221 to +249
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

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)

export async function resolvePresignedPacketUrl(
url: string,
tracer?: TriggerTracer
Expand Down
32 changes: 32 additions & 0 deletions tests/e2e/ioSerialization.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { stringifyIO, parsePacket } from "./ioSerialization";

describe("IO Serialization", () => {
it("should serialize and deserialize Blob objects", async () => {
const blob = new Blob(["Hello, world!"], { type: "text/plain" });
const serialized = await stringifyIO(blob);
expect(serialized.dataType).toBe("application/blob");

const deserialized = await parsePacket(serialized);
expect(deserialized instanceof Blob).toBe(true);
const text = await deserialized.text();
expect(text).toBe("Hello, world!");
});

it("should serialize and deserialize JSON objects", async () => {
const obj = { key: "value" };
const serialized = await stringifyIO(obj);
expect(serialized.dataType).toBe("application/super+json");

const deserialized = await parsePacket(serialized);
expect(deserialized).toEqual(obj);
});

it("should serialize and deserialize strings", async () => {
const str = "Hello, world!";
const serialized = await stringifyIO(str);
expect(serialized.dataType).toBe("text/plain");

const deserialized = await parsePacket(serialized);
expect(deserialized).toBe(str);
});
});