Fix/build errors - flux-client.ts #5
Conversation
- Fix type assertion for createGrpcClient return type - Fix async iterator type for messageStream bidi stream - All TypeScript errors resolved, build passes
|
Only |
There was a problem hiding this comment.
Pull request overview
This PR addresses TypeScript build errors in flux-client.ts by adding type assertions to resolve compilation issues with gRPC client creation and stream handling. The changes include formatting fixes (adding newlines at end of files) and two type assertions aimed at satisfying TypeScript's type checker.
Key Changes
- Added type assertion for
createGrpcClientreturn value usingas unknown aspattern - Added type assertion for bidirectional gRPC stream with single-element tuple destructuring
- Added trailing newlines to
index.tsandagent-loader.tsfor consistency
Reviewed changes
Copilot reviewed 1 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/flux-client.ts | Added type assertions to fix TypeScript compilation errors for gRPC client initialization and message stream iteration |
| src/index.ts | Added trailing newline at end of file |
| src/agent-loader.ts | Added trailing newline at end of file |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }); | ||
|
|
||
| this.client = await createGrpcClient(GRPC_SERVER_ADDRESS, clientImpl); | ||
| this.client = (await createGrpcClient(GRPC_SERVER_ADDRESS, clientImpl)) as unknown as Awaited<ReturnType<typeof createGrpcClient>>; |
There was a problem hiding this comment.
This type assertion is redundant and potentially problematic. The expression casts the result of createGrpcClient to Awaited<ReturnType<typeof createGrpcClient>>, which is the same type it already returns. Using as unknown as to cast something to its own type suggests a workaround for a type system issue that should be addressed differently.
Looking at line 9, the client property is already typed as Awaited<ReturnType<typeof createGrpcClient>> | null, and auth.ts uses createGrpcClient without any type assertions (lines 80, 88, 113, 170, 187). The assertion should be removed entirely, allowing TypeScript to infer the correct type naturally.
| this.client = (await createGrpcClient(GRPC_SERVER_ADDRESS, clientImpl)) as unknown as Awaited<ReturnType<typeof createGrpcClient>>; | |
| this.client = await createGrpcClient(GRPC_SERVER_ADDRESS, clientImpl); |
| (async () => { | ||
| for await (const [message] of this.client!.FluxService.messageStream) { | ||
| const stream = this.client!.FluxService.messageStream; | ||
| for await (const [message] of stream as AsyncIterable<[IncomingMessage | { ack: string }]>) { |
There was a problem hiding this comment.
The destructuring pattern and type assertion here appear unusual. The type assertion declares that the stream yields tuples [IncomingMessage | { ack: string }], but this single-element tuple pattern is unconventional.
Typically, bidirectional gRPC streams would either:
- Yield values directly:
for await (const message of stream as AsyncIterable<IncomingMessage | { ack: string }>) - Yield multi-element tuples if there's metadata:
for await (const [message, metadata] of stream)
Verify that the better-grpc library actually yields single-element tuples, or simplify the destructuring if the tuple wrapper is unnecessary.
| for await (const [message] of stream as AsyncIterable<[IncomingMessage | { ack: string }]>) { | |
| for await (const message of stream as AsyncIterable<IncomingMessage | { ack: string }>) { |
|
Thanks! This is really helpful. We’ll definitely merge it if this build issue starts showing up more. |
|
The client should already be properly typed, and adding a type assertion here would cause issues in the type system. @adityajha2005 would you mind opening an issue for this instead? |
Sure, that makes sense. I’ll open an issue with the details and reproduction steps. |
3 participants
Summary
This PR fixes TypeScript build errors in
flux-clientand ensures the project builds cleanly without type errors.What’s fixed
createGrpcClientmessageStreambidirectional streamResult
Notes
Build status
Before
After
