Skip to content
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

Migrate RPC to ConnectRPC #135

Merged
merged 14 commits into from
Jan 9, 2024
Merged

Migrate RPC to ConnectRPC #135

merged 14 commits into from
Jan 9, 2024

Conversation

chacha912
Copy link
Contributor

@chacha912 chacha912 commented Dec 15, 2023

What this PR does / why we need it?

Migrate RPC to ConnectRPC.
For more information about this migration, follow:

Any background context you want to provide?

  • Add error_details.proto
// NOTE(chacha912): This code is based on the protobuf definitions from the file located at:
// https://buf.build/googleapis/googleapis/file/main:google/rpc/error_details.proto.
// The protobuf definitions are compiled into an SDK, which can be installed via npm.
// (SDK Source: https://buf.build/googleapis/googleapis/sdks/main)
//
// However, during testing, we encountered an error due to the use of ESM import/export syntax.
// To address this issue, we use the 'js_import_style=legacy_commonjs' option and manually build
// the protobuf, rather than using the SDK directly.
// (For more details, refer to: https://github.com/bufbuild/protobuf-es/issues/587)
  • JSDOM Environment Patch for TextEncoder and TextDecoder
// NOTE(chacha912): This patches the global objects TextEncoder, TextDecoder
// which are missing in the JSDOM environment.
// See https://github.com/jsdom/jsdom/issues/2524 for more details.
  • Introduction of Custom Thunk
    • To handle common processing for asynchronous actions, a custom thunk has been introduced. For more information, please refer to the documentation in the rpc-error-handling.md.

What are the relevant tickets?

Fixes #

Checklist

  • Added relevant tests or not required
  • Didn't break anything

@chacha912 chacha912 marked this pull request as ready for review December 19, 2023 00:35
@chacha912 chacha912 requested a review from hackerwins December 19, 2023 00:37
Copy link
Member

@hackerwins hackerwins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution.

When I tested this PR in my local environment, all features worked fine. Next, we need to update the documents below:

  • README.md
  • design/rpc-error-handling.md

@chacha912 chacha912 marked this pull request as draft December 20, 2023 04:01
@chacha912 chacha912 marked this pull request as ready for review January 1, 2024 12:19
@chacha912 chacha912 requested a review from hackerwins January 1, 2024 12:20
@hackerwins hackerwins merged commit 2385a69 into main Jan 9, 2024
2 checks passed
@hackerwins hackerwins deleted the connectrpc branch January 9, 2024 07:52
@hackerwins
Copy link
Member

Thanks for your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants