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

feat: grpc-web support promise stream #631

Closed
wants to merge 8 commits into from
Closed

Conversation

luhuaei
Copy link
Contributor

@luhuaei luhuaei commented Jul 28, 2022

No description provided.

paralin added 3 commits July 27, 2022 20:06
Signed-off-by: Christian Stewart <[email protected]>
"responsePromiseOrObservable" was changed to return the Observable type if
clientStreaming is set:

stephenh@d3e7f1f#r79607248

This is only the case when using grpc-web.

Fixes stephenh#628

Signed-off-by: Christian Stewart <[email protected]>
const inputType = requestType(ctx, methodDesc, partialInput);
params.push(code`request: ${inputType}`);
// grpc-web client stream or bidirectional stream not request params when use Promise.
if (!options.returnObservable && options.outputClientImpl === 'grpc-web' && methodDesc.clientStreaming) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just invert the if statement here.

if (options.returnObservable || options.outputClientImpl !== "grpc-web" || !methodDesc.clientStreaming)

Copy link
Collaborator

Choose a reason for hiding this comment

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

"grpc-web client stream or bidirectional stream not request params when use Promise."

I can't understand this sentence, could you make it more clear?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When using the Promise return type, the input parameter does not require the request parameter.

See https://github.com/stephenh/ts-proto/pull/631/files#diff-cf75bd2b26f57126e3c440377e55dfa55c406359a04d2029355db727cac33c3eR44

In this implementation, the request parameter is not required. Use callback to get the relevant data stream, use the write method to send data streams.

src/generate-services.ts Show resolved Hide resolved
@paralin
Copy link
Collaborator

paralin commented Jul 29, 2022

@luhuaei Are you still working on this? Am happy to help with getting it ready to merge.

@luhuaei
Copy link
Contributor Author

luhuaei commented Jul 30, 2022

@luhuaei Are you still working on this? Am happy to help with getting it ready to merge.

@paralin thanks!

I make other pull request #633 to support grpc-web stream method.

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.

2 participants