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

grpc-web messages have an extra toObject method #636

Closed
lededje opened this issue Aug 3, 2022 · 4 comments · Fixed by #728
Closed

grpc-web messages have an extra toObject method #636

lededje opened this issue Aug 3, 2022 · 4 comments · Fixed by #728
Labels
enhancement New feature or request released

Comments

@lededje
Copy link

lededje commented Aug 3, 2022

I'm using redux to manage state and need the responses to be serializeable. I found the flag to change dates into strings which solved half the issue, now it complains that toObject is in every response but it's not in the generated types. How best should I work around this?

@stephenh
Copy link
Owner

stephenh commented Aug 6, 2022

@lededje normally ts-proto decode-d data does not have any methods, so would be perfect to put into redux, however, for grpc-web, we currently have to add it here:

  responseType: {
    deserializeBinary(data: Uint8Array) {
      return {
        ...DashCred.decode(data),
        toObject() {
          return this;
        },
      };
    },

Because of how the improbable-eng/grpc-web library works.

If you can play with the generated code, and get a deserializeBinary working that doesn't need this toObject hack, that'd be great!

(Maybe there is a feature/flag in the the improbable-eng library that means we don't need toObject, or maybe you could submit a PR to improbable-eng that would make it detect toObject-is-not-available and just use the return value of deserializeBinary as-is.)

(Also, as a disclaimer, this toObject approach is a little old at this point, so maybe it could be something that is already fixed / not necessary in the latest versions of improbable-eng/grpc-web.)

The wrinkle is that I'm not personally using grpc-web at this point, so you'll have to poke around a bit to figure out a fix, but let us know what you find! Thanks!

@lededje
Copy link
Author

lededje commented Sep 4, 2022

Thanks for the explaination. I think I'm going to stick with a function that strips that function out when I get a response.

Happy for you to close this.

@stephenh stephenh changed the title Responses are not serializible grpc-web messages have an extra toObject method Sep 4, 2022
@stephenh stephenh added the enhancement New feature or request label Sep 4, 2022
@stephenh
Copy link
Owner

stephenh commented Sep 4, 2022

Hey @lededje sure, np, I'll leave the issue open just for others to find & maybe someone can find a way to fix it.

relaxgo added a commit to relaxgo/ts-proto that referenced this issue Dec 11, 2022
relaxgo added a commit to relaxgo/ts-proto that referenced this issue Dec 11, 2022
stephenh pushed a commit that referenced this issue Dec 12, 2022
## [1.135.3](v1.135.2...v1.135.3) (2022-12-12)

### Bug Fixes

* return grpc-web response without toObject method ([#728](#728)) ([7431aa8](7431aa8)), closes [#636](#636)
@stephenh
Copy link
Owner

🎉 This issue has been resolved in version 1.135.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants