fix/tschema field order#66
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a field ordering bug in the TSchema.Struct encode function that caused fields to be incorrectly ordered during CBOR encoding when using NullOr/UndefinedOr wrappers. The fix ensures that field order matches the schema definition order by using Object.keys(fields) instead of iterating over the encoded struct's properties.
Key Changes:
- Fixed field ordering in
Structencode function by using schema definition order (Object.keys(fields)) instead of runtime object order - Added regression test to verify field order preservation with NullOr fields
- Converted
StructandVariantfrom function declarations to arrow function expressions
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/evolution/src/core/TSchema.ts | Core fix to preserve field order in Struct encoding; converted Struct and Variant to arrow functions |
| packages/evolution/test/TSchema.test.ts | Added regression test for field ordering with NullOr fields |
| packages/evolution/docs/modules/core/TSchema.ts.md | Updated type signatures for Struct and Variant to reflect arrow function syntax |
| docs/content/docs/modules/core/TSchema.mdx | Updated Variant type signature to match source code changes |
| docs/next-env.d.ts | Auto-generated Next.js file that should not be committed (in .gitignore) |
| .changeset/chilly-brooms-end.md | Documented the bug fix with before/after examples |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const fieldEntries = Object.entries(encodedStruct).filter(([key]) => key !== detectedTagField) | ||
| const fieldValues = fieldEntries.map(([_, value]) => value) as ReadonlyArray<Data.Data> | ||
| // Use Object.keys(fields) to preserve schema definition order | ||
| // (Object.entries doesn't guarantee property order) |
There was a problem hiding this comment.
The comment is technically inaccurate. Since ES2015, Object.entries() does guarantee property order for string keys (insertion order is preserved). The real issue is that you need to ensure the order comes from the schema definition (fields) rather than the runtime object (encodedStruct). Consider revising to: "Use Object.keys(fields) to preserve schema definition order"
| // (Object.entries doesn't guarantee property order) | |
| // (Preserve the order as defined in the schema, not the runtime object) |
No description provided.