-
Notifications
You must be signed in to change notification settings - Fork 76
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
Switch to v2 #948
Merged
Switch to v2 #948
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Remove the toString method, and let GeneratedFile.jsDoc() return a Printable. This brings the JSDocBlock printable in line with others.
And provide utilities for Timestamp, such as timestampNow()
Add `toBinary` and `fromBinary` functions to serialize to and from protobuf encoded bytes. Added compatibility tests with v1 for now.
This annotation informs bundlers that the succeeding function call is free of side effects. This means the symbol can be removed from the module during tree-shaking if it is unused. See #470 The annotation is not terribly effective yet, since all generated descriptors depend on the file descriptor, but it's still worthwhile.
This adds two new types to the reflect API: ReflectList and ReflectMap. The API now always returns messages as ReflectMessage, list fields as ReflectList, and map fields as ReflectMap. Note that accessing an unset message fields returns a ReflectMessage with a new zero-message. The methods ReflectMessage.addListItem and ReflectMessage.setMapEntry are kept for now. We can decide whether it's worth to keep them for the hot path when we take a look at performance. This also updates create() to fully support the "init" argument.
The IEEE standard requires that NaN != NaN. It's questionable whether that should also be the case when comparing two protobuf fields,
The message descriptor contains cyclic references and methods, causing issues with Jest, RSC serialization, structured clone, and others. By removing the property, messages are no longer self-describing, and functions that need schema information require the user to path both the message descriptor and the message.
Most importantly, tests for serializing map fields. Fixes a bug in fromBinary, where map keys were converted to strings, which are the correct representation in a message, but not for a ReflectMap. This bug was most likely introduced in 7054ed7, which limited the conversion to a smaller set of acceptable types.
In case of a test failure, we'll see the diff instead of Jest crashing. See jestjs/jest#11617 (comment) There don't appear to be any adverse effects of enabling the option. Test failures when strictly comparing message classes are still reported correctly.
In order to reduce the number of main exports, move types that are rarely needed by end-users to a new subpath "wire". - BinaryWriter and BinaryReader have been moved, and their interfaces discarded. - Users can bring their own Text encoding API via wire/configureTextEncoding() now instead of the serialization options readerFactory / writerFactory. - protoBase64 has been split into the functions base64Decode and base64Encode, with the latter allowing to encode with padding, without padding, or URL-safe. The exports protoDelimited and protoBase64 are also candidates for moving to wire, but are not moved by this commit.
Move reflect/wkt.ts to wkt/wrappers.ts to keep WKT-specific code in one place. This does not move the code for custom JSON representation of WKTs.
fileDesc() from codegenv1/hydrate.ts restored redundant json names incorrectly, leading DescField.jsonName to be an empty string instead of undefined.
Signed-off-by: Timo Stamm <[email protected]> Co-authored-by: Carol Gunby <[email protected]> Co-authored-by: Steve Ayers <[email protected]>
Signed-off-by: Timo Stamm <[email protected]> Co-authored-by: Timo Stamm <[email protected]>
Co-authored-by: Carol Gunby <[email protected]>
srikrsna-buf
approved these changes
Jul 30, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This merges the branch v2 into
main
. A copy ofmain
is kept in the branch v1.