-
Notifications
You must be signed in to change notification settings - Fork 15.6k
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
JS: parse (un)packed fields conditionally #7379
Conversation
This comment has been minimized.
This comment has been minimized.
76462d0
to
51b08e8
Compare
@TeBoring I removed the relevant lines from |
@TeBoring we're occasionally hit by this problem (since our JavaScript client communicates with Ruby servers, where the current implementation always encodes repeated fields unpacked). It's great if you have time to look at this. |
@haberman sorry for the sudden mention, but could you or someone else look at this? The diff is essentially about 30 lines and it should be relatively easy to review... We're actively applying changes to our internal protobuf messages which are sent between JS and Ruby, and #1701 is a real annoyance since failing to add |
@haberman thanks for reviewing and kicking the CI! The only failures are Java things (which I think is unrelated to this PR) and the lack of language label (which I cannot help), so I guess there's nothing I can do now. Right? |
@haberman could you merge this? |
Thank you for your contributions! |
The current JS implementation doesn't comply with the following criteria:
In this PR, the parser checks
WireType
before parsing packable fields, and then always append to the existing array, instead of replacing it.Fixes #1701.