Conversation
`codec.encode` previously executed two passes for every query parameter and mutation value: first calling `encodeValue` to construct an intermediate JavaScript representation, and then passing that to `GrpcService.encodeValue_`. `GrpcService.encodeValue_` allocated a new `ObjectToStructConverter` instance and an internal `Set` for each value, using `.map()` and bound closures for array transformations. This change converts `codec.encode` to emit `google.protobuf.Value` messages directly in a single pass: - Primitives dispatch directly via a `typeof` switch. - Array members are converted in an indexed loop into a pre-sized array without intermediate collections or closures. - The import of `GrpcService` is removed from `src/codec.ts`, eliminating the dependency on `./common-grpc/service`. - Encoding rules for Spanner types are preserved: finite non-integers encode as numbers; integers, `NaN`, and `±Infinity` encode as strings; and wrapped `Float`/`Float32` instances retain their numeric representation. - Cross-realm `Date` instances (e.g. from `vm` contexts) are handled via a tag check fallback on the object path. - Sparse arrays now fail immediately on missing indices rather than producing arrays with holes that fail during protobuf serialization.
There was a problem hiding this comment.
Code Review
This pull request refactors the encoding logic in codec.ts to directly emit the google.protobuf.Value shape in a single pass, removing the dependency on GrpcService and improving performance. It also updates and expands the test suite in test/codec.ts to verify the new encoding behavior across various data types and edge cases. Feedback on the changes suggests reordering the type checks in encodeObject to handle boxed primitives and cross-realm dates before generic objects, preventing potential serialization issues.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the encode function in codec.ts to directly emit the google.protobuf.Value shape in a single pass, removing the dependency on GrpcService and improving performance by avoiding intermediate allocations. The test suite in test/codec.ts has been updated to remove GrpcService stubbing and expanded with comprehensive test cases covering various data types, nested structures, boxed primitives, and error handling for unsupported values. There are no review comments to address, and I have no further feedback to provide.
codec.encodepreviously executed two passes for every query parameter and mutation value: first callingencodeValueto construct an intermediate JavaScript representation, and then passing that toGrpcService.encodeValue_.GrpcService.encodeValue_allocated a newObjectToStructConverterinstance and an internalSetfor each value, using.map()and bound closures for array transformations.This change converts
codec.encodeto emitgoogle.protobuf.Valuemessages directly in a single pass:typeofswitch.GrpcServiceis removed fromsrc/codec.ts, eliminating the dependency on./common-grpc/service.NaN, and±Infinityencode as strings; and wrappedFloat/Float32instances retain their numeric representation.Dateinstances (e.g. fromvmcontexts) are handled via a tag check fallback on the object path.