-
Notifications
You must be signed in to change notification settings - Fork 62
feat: add ignoreUnknownFields to protobuf JSON parsing #63
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| /** | ||
| * Test file to demonstrate that the protobuf-utils wrapper handles unknown fields gracefully. | ||
| * | ||
| * This test can be run manually to verify the fix. Since the project doesn't have | ||
| * a test runner configured, this serves as documentation of the expected behavior. | ||
| * | ||
| * To test manually: | ||
| * 1. Add a new field to a protobuf schema on the backend | ||
| * 2. Deploy the backend | ||
| * 3. Use an older version of the webapp (without regenerating protobuf files) | ||
| * 4. Verify that the webapp doesn't crash when receiving the new field | ||
| */ | ||
|
|
||
| import { fromJson } from "./protobuf-utils"; | ||
| import { MessageSchema } from "../pkg/gen/apiclient/chat/v2/chat_pb"; | ||
|
|
||
| /** | ||
| * Example: Testing that fromJson ignores unknown fields | ||
| * | ||
| * This would simulate a backend returning a message with a new field | ||
| * that doesn't exist in the current schema. | ||
| */ | ||
| function testIgnoreUnknownFields() { | ||
| // Simulate JSON response from backend with an extra field "newField" | ||
| const jsonWithUnknownField = { | ||
| messageId: "test-123", | ||
| payload: { | ||
| user: { | ||
| content: "Hello", | ||
| selectedText: "", | ||
| newFieldThatDoesntExistYet: "This is a new field from a newer backend version", | ||
| }, | ||
| }, | ||
| timestamp: "0", | ||
| }; | ||
|
|
||
| try { | ||
| // This should NOT throw an error even though "newFieldThatDoesntExistYet" doesn't exist in the schema | ||
| const message = fromJson(MessageSchema, jsonWithUnknownField); | ||
| console.log("✓ Successfully parsed message with unknown field"); | ||
| console.log(" Message ID:", message.messageId); | ||
| console.log(" User content:", message.payload.user?.content); | ||
| return true; | ||
| } catch (error) { | ||
| console.error("✗ Failed to parse message with unknown field:", error); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Example: Testing that fromJson still validates required fields | ||
| */ | ||
| function testRequiredFieldsStillValidated() { | ||
| // Missing required messageId field | ||
| const invalidJson = { | ||
| payload: { | ||
| user: { | ||
| content: "Hello", | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| try { | ||
| const message = fromJson(MessageSchema, invalidJson); | ||
| console.log("✓ Parsed message (messageId will be empty string):", message.messageId); | ||
| return true; | ||
| } catch (error) { | ||
| console.error("✗ Failed to parse message:", error); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| // Export test functions for manual testing | ||
| export { testIgnoreUnknownFields, testRequiredFieldsStillValidated }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| import { DescMessage, fromJson as bufFromJson, JsonValue } from "@bufbuild/protobuf"; | ||
|
|
||
| /** | ||
| * Wrapper around fromJson that ignores unknown fields to prevent crashes | ||
| * when new fields are added to the schema. | ||
| * | ||
| * This allows forward compatibility - older webapp versions can work with | ||
| * newer backend versions that introduce new fields. | ||
| */ | ||
| export function fromJson<Desc extends DescMessage>( | ||
| schema: Desc, | ||
| json: JsonValue, | ||
| ): InstanceType<Desc["message"]> { | ||
| return bufFromJson(schema, json, { | ||
| ignoreUnknownFields: true, | ||
| }); | ||
|
Comment on lines
+10
to
+16
|
||
| } | ||
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 test file contains only exported functions but no executable tests. Since the project doesn't have a test runner configured (no test scripts in package.json, no jest/vitest/mocha dependencies), this file serves as documentation rather than automated tests. Consider either: 1) setting up a test runner and converting these to actual test cases, or 2) renaming the file to something like
protobuf-utils.examples.tsor moving the documentation to a comment/doc file to better reflect its current purpose as manual testing documentation.