Conversation
…form Co-authored-by: Rezrazi <2086576+Rezrazi@users.noreply.github.com>
…s platform Co-authored-by: Rezrazi <2086576+Rezrazi@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This pull request adds response streaming support to the TypeScript OpenAPI Generator for Node.js environments. The implementation enables access to the underlying response stream from undici's fetch implementation while maintaining compatibility with existing code.
Changes:
- Added
stream()method to theResponseBodyinterface for Node.js platform - Implemented
stream()inIsomorphicFetchHttpLibraryto expose undici's response body stream - Provided null-returning
stream()implementation inSelfDecodingBodyfor non-streaming contexts
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/openapi-generator/src/main/resources/typescript/http/http.mustache | Adds stream() method to ResponseBody interface and SelfDecodingBody class for Node.js platform |
| modules/openapi-generator/src/main/resources/typescript/http/isomorphic-fetch.mustache | Implements stream() to return resp.body from undici's Response |
| samples/openapi3/client/petstore/typescript/builds/default/http/http.ts | Generated sample with required stream() method |
| samples/openapi3/client/petstore/typescript/builds/default/http/isomorphic-fetch.ts | Generated sample implementing stream access |
| samples/openapi3/client/petstore/typescript/builds/object_params/http/http.ts | Generated sample with optional stream?() method (inconsistent) |
| samples/openapi3/client/petstore/typescript/builds/object_params/http/isomorphic-fetch.ts | Generated sample implementing stream access |
| samples/openapi3/client/petstore/typescript/builds/inversify/http/http.ts | Generated sample with optional stream?() method (inconsistent) |
| samples/openapi3/client/petstore/typescript/builds/inversify/http/isomorphic-fetch.ts | Generated sample implementing stream access |
| samples/openapi3/client/petstore/typescript/builds/browser/http/http.ts | Browser build correctly excludes stream method |
| samples/client/echo_api/typescript/build/http/http.ts | Generated sample with optional stream?() method (inconsistent) |
| samples/client/echo_api/typescript/build/http/isomorphic-fetch.ts | Generated sample implementing stream access |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export interface ResponseBody { | ||
| text(): Promise<string>; | ||
| binary(): Promise<Buffer>; | ||
| stream?(): ReadableStream<Uint8Array> | null; |
There was a problem hiding this comment.
The stream method is marked as optional in the ResponseBody interface, but it's implemented as a required method in the SelfDecodingBody class. This creates an inconsistency with the template definition in http.mustache (line 221) which defines it as required. The interface should have stream() without the question mark to match the template and ensure all implementations provide this method.
| stream?(): ReadableStream<Uint8Array> | null; | |
| stream(): ReadableStream<Uint8Array> | null; |
| export interface ResponseBody { | ||
| text(): Promise<string>; | ||
| binary(): Promise<Buffer>; | ||
| stream?(): ReadableStream<Uint8Array> | null; |
There was a problem hiding this comment.
The stream method is marked as optional in the ResponseBody interface, but it's implemented as a required method in the SelfDecodingBody class. This creates an inconsistency with the template definition in http.mustache (line 221) which defines it as required. The interface should have stream() without the question mark to match the template and ensure all implementations provide this method.
| stream?(): ReadableStream<Uint8Array> | null; | |
| stream(): ReadableStream<Uint8Array> | null; |
| export interface ResponseBody { | ||
| text(): Promise<string>; | ||
| binary(): Promise<Buffer>; | ||
| stream?(): ReadableStream<Uint8Array> | null; |
There was a problem hiding this comment.
The stream method is marked as optional in the ResponseBody interface, but it's implemented as a required method in the SelfDecodingBody class. This creates an inconsistency with the template definition in http.mustache (line 221) which defines it as required. The interface should have stream() without the question mark to match the template and ensure all implementations provide this method.
| stream?(): ReadableStream<Uint8Array> | null; | |
| stream(): ReadableStream<Uint8Array> | null; |
| } | ||
|
|
||
| {{#platforms}} | ||
| {{#node}} |
There was a problem hiding this comment.
The stream method implementation always returns null in SelfDecodingBody. Consider adding a comment explaining that this implementation doesn't support streaming and that streaming is only available through the actual fetch response body in IsomorphicFetchHttpLibrary. This would help developers understand when they can expect a usable stream versus null.
| {{#node}} | |
| {{#node}} | |
| // SelfDecodingBody does not support streaming. A readable stream is only | |
| // available from the underlying fetch Response body in IsomorphicFetchHttpLibrary. |
isomorphic-fetch.mustacheto addstream()function for Node.js platformhttp.mustacheto addstreammethod toResponseBodyinterface for Node.jsstream()implementation toSelfDecodingBodyclass for Node.js (returns null)stream?()(optional) tostream()(required) per user feedbackOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.