Skip to content

grpc-js: Add end(md?: Metadata) method to streaming server calls #1525

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

Merged

Conversation

murgatroid99
Copy link
Member

This fixes #1520.

@@ -255,6 +258,15 @@ export class ServerDuplexStreamImpl<RequestType, ResponseType> extends Duplex
sendMetadata(responseMetadata: Metadata): void {
this.call.sendMetadata(responseMetadata);
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
end(metadata?: any) {

Choose a reason for hiding this comment

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

I know this is a copy from below but shouldn't this be metadata?: Metadata ?
this.trailingMetadata is of type Metadata so I can't imagine what other type it could be.

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is that TypeScript doesn't like having a class that extends Duplex but implements end with a type that is incompatible with the end method declared in the Duplex class. Declaring the argument as any smooths that out.

There's a more complicated way of doing this that I use in object-stream.ts where I declare an intermediate type that uses any & T as the type, and then I can subtype that using T. I don't think that's valuable here though because this type is only in the implementation. The interface that actually gets provided to the user has the stricter Metadata type, and that's what matters here.

Choose a reason for hiding this comment

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

@murgatroid99 The other day I came across a similar situation and was able to mitigate it with Omit<Type, 'end'>. Might be useful in the future.

@murgatroid99 murgatroid99 merged commit 68b0609 into grpc:master Aug 19, 2020
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.

grpc-js: ServerWritableStream.end(metadata?: Metadata) is missing from type definition
3 participants