Skip to content
This repository was archived by the owner on Nov 18, 2025. It is now read-only.

feat: use proto3 JSON serializer for REGAPIC workflows#1074

Merged
alexander-fenster merged 2 commits intomasterfrom
use-serializer
Aug 5, 2021
Merged

feat: use proto3 JSON serializer for REGAPIC workflows#1074
alexander-fenster merged 2 commits intomasterfrom
use-serializer

Conversation

@alexander-fenster
Copy link
Contributor

In this pull request, I'm adding a feature to use the new proto3 JSON serializer in REGAPIC workflows instead of .toObject and .fromObject, and also refactoring the very hard-to-read src/fallback.ts into several files which now, I believe, have much more readable structure.

In this PR, we:

  • in REST transport, use .toProto3JSON and .fromProto3JSON instead of .toObject/.fromObject, so we support all the fancy JSON serialization rules;
  • stop using protobuf.js "rpcImpl" mechanism, instead generating our own service stub in a straightforward way;
  • extract the fetch logic into a separate file;
  • extract the transport-related encoding and decoding of requests and responses into two separate files (one for REST, another one for proto-over-HTTP).

There is almost no new code, the refactor is mostly the existing code moved around.

In general, I hope the new setup is more readable.

@alexander-fenster alexander-fenster requested a review from a team as a code owner August 4, 2021 19:17
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Aug 4, 2021
@summer-ji-eng
Copy link
Contributor

Thank you so much for refactor the fallback.ts.
It makes the fallback more readable. 💯

@alexander-fenster alexander-fenster merged commit 6ef89f1 into master Aug 5, 2021
@alexander-fenster alexander-fenster deleted the use-serializer branch August 5, 2021 04:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants