Comments (16)
nice @toonvanstrijp , yep i knew I overlooked that. Thanks!
from ts-proto.
I agree with pretty much everything in #60 :-) The HeroServiceControllerMethods
in particular is a great ergonomics trick. Next time I want to play with grpc streaming, I will be using this nestjs setup. :-)
from ts-proto.
Great work @toonvanstrijp
Somebody reported an issue to me last night actually, I don't know if yours fixes this or not.
Here reported this in the nestjs discord channel
i see the --ts_proto_opt=nestJs=true and i used it. beautiful.
functions are lowercased. however, empty still appears for responses that should be void.
am I missing an option for this?
I still need to confirm it but I must admit I think I overlooked that :-(
If you want I can take a look after you have merged? Or you can - let me know either way.
from ts-proto.
Hi @toonvanstrijp yeah, good call out. I've not actually used streams in protobuf on node; I did awhile ago in some protobuf on JVM but kinda forgot about streaming for node so far. :-)
I think your initial fix is a good first step...
Since you're an NestJS user, can you extend the nestjs-simple
integration test that @iangregsondev just added:
https://github.com/stephenh/ts-proto/tree/master/integration/nestjs-simple
To have an additional .proto
message/method with stream
in it?
And then run the update-bins.sh
and codegen.sh
to update the nestjs-simple
output to ensure it works the way you expect, i.e. most things will still be Promise
but the stream will be Observable
.
If we have that, I think that's a good enough to accept. I.e. if it works for you and NestJS, I'm fine just having a known-issue that streaming is still unsupported for non-NestJS use cases.
Two things:
- fwiw this example:
@GrpcStreamCall()
bidiHello(requestStream: any) {
requestStream.on('data', message => {
console.log(message);
requestStream.write({
reply: 'Hello, world!'
});
});
}
Makes me think that, if you use streams (maybe as input parameters?), the service interface is no longer 100% the same for client and server, b/c the server-side method accepts a grpc.ServerReadableStream
and the client-side method accepts a ...ReplaySubject
/subject. (FYI @iangregsondev ).
- I would love to have the
nestjs-simple
(or similar) integration test have a unit test that actual standups a NestJS server on local port whatever, instantiates a NestJS client, and watches the client be able to make calls to the server (again FYI @iangregsondev , I think this would be a great "tutorial" for NestJS users + a really great regression test to include in ts-proto to make sure we know "for sure" things owkr).
At first this test could be just for regular synchronous calls, but then it'd be great especially for these streaming methods, given the APIs can be fairly unintuitive at first.
But I'm fine punting on this #2 ask if you don't want to tackle it in the 1st "return
from ts-proto.
Great catch @toonvanstrijp , I must admit I don't use the streaming option so I kind of never investigated it - but your fix looks good.
I kind of always set everything to Observable :-) Although that's a mistake on my part as others might not want this and then the stream part would fail.
@stephenh I believe the client/server API interface can be the same, in-fact that's the way I would use it. Returns an observable and accepts an observable - I think these both work on client/server.
The thing is that the implementation supports way more stuff :-)
Call stream handler#
When the method return value is defined as stream, the @GrpcStreamCall() decorator provides the function parameter as grpc.ServerDuplexStream, which supports standard methods like .on('data', callback), .write(message) or .cancel().
So it kind of supports a call back on the client according to the nestjs docs.
I must admit, I don't use it, so I am not the best person to comment.
@toonvanstrijp Can you confirm.
Maybe this callback is a kind of edge cases for most systems and not needed or used minimal? Not sure, but if this is the case - then maybe we can say - if you require a custom implementation that doesn't use the generated interface and create it manually - if it's only for 1 or 2 methods.
Again - I am not sure the right approach here.
Cheers!
from ts-proto.
@stephenh @iangregsondev
Woah you guys are fast! Thanks for your replies.
First of all, yes I'll add a test for this inside nestjs-simple
.
- Then there is the
@GrpcStreamCall()
I think this one does the same as returning / accepting an observable. I thinknestjs
add theObservable
support cause it seems a bit more elegant in thenestjs framework
.
As I said I'm quite new to this but we could add an option inside the proto file so you can tell if the rpc call
needs to be formatted confirm the Observable
pattern or in the GrpcStreamCall
way?
extend google.protobuf.MethodOptions {
optional bool native_grpc_stream_call = 50000;
}
I'm not sure if this is the right way to make this possible, so if you guys have other ideas let me know ;)
from ts-proto.
@stephenh I'm trying to run the update-bins.sh
but I'm missing this tool: protoc-gen-dump
. How do I set up the development environment?
from ts-proto.
@stephenh I'm trying to run the
update-bins.sh
but I'm missing this tool:protoc-gen-dump
. How do I set up the development environment?
Did you follow this in the readme ? I also had some initial problems but i think it was fixed following this.
After running yarn install (which will fail in yarn test on the first time), run ./pbjs.sh to create the bootstrap types, and ./integration/pbjs.sh to create the integration test types. These pbjs-generated files are not currently checked in.
After this the tests should pass.
After making changes to ts-proto, you can run cd integration and ./codegen.sh to re-generate the test case *.ts output files that are in each integration// directory.
from ts-proto.
@iangregsondev ow haha I was running it from the wrong directory. I was doing this: ./integration/update-bins.sh
but I need the cd
into integration
and then run ./update-bins.sh
from ts-proto.
Alrighty I'm making some progress and found out that there is a difference between the client / server implementation of nestjs
.
If we take the following proto:
service HeroService {
rpc FindOneHero (HeroById) returns (Hero) {}
rpc FindOneVillain (VillainById) returns (Villain) {}
rpc FindManyVillain (stream VillainById) returns (stream Villain) {}
}
And we generate the ts file:
export interface HeroService {
findOneHero(request: HeroById): Promise<Hero>;
findOneVillain(request: VillainById): Promise<Villain>;
findManyVillain(request: Observable<VillainById>): Observable<Villain>;
}
The interface is correct for server implementation. Hover for nestjs
it doesn't matter if you return an Observable
or Promise
(only when using streams). When nestjs
receives a Promise
it'll transform this in the client to an Observable
.
So the server typescript could look like this:
export interface HeroServiceServer {
findOneHero(request: HeroById): Promise<Hero> | Observable<Hero> | Hero;
findOneVillain(request: VillainById): Promise<Villain> | Observable<Villain> | Villain;
findManyVillain(request: Observable<VillainById>): Observable<Villain>;
}
This way it matches the nestjs
format, since nestjs
is taking care of the transformation between different types.
Then the client typescript file should look like this:
export interface HeroServiceClient {
findOneHero(request: HeroById): Observable<Hero>;
findOneVillain(request: VillainById): Observable<Villain>;
findManyVillain(request: Observable<VillainById>): Observable<Villain>;
}
I think when nestJs=true
we need to create those two different server
and client
interfaces
so that it matches the nestjs
way of how to integrate this.
What is your opinion on this?
from ts-proto.
@stephenh @iangregsondev
also I added some tests!
Take a look! debugged@7213e8b
As you can see right now I'm doing a ugly type casting here: debugged@7213e8b#diff-f54c909de34198d5f4758846c62e72efR41
But we can solve this by implementing the solution I provided above.
from ts-proto.
The interface is correct for server implementation. Hover for
nestjs
it doesn't matter if you return anObservable
orPromise
(only when using streams). Whennestjs
receives aPromise
it'll transform this in the client to anObservable
.
Yes true, nestjs will convert this to standard object as its going over the wire. There is no such thing as an observable when going over the wire.
The main thing it gives you is how to deal with your code inside the service methods, dealing with ASYNC and AWAIT, pure promises, observables or simple objects.
So the server typescript could look like this:
export interface HeroServiceServer { findOneHero(request: HeroById): Promise<Hero> | Observable<Hero> | Hero; findOneVillain(request: VillainById): Promise<Villain> | Observable<Villain> | Villain; findManyVillain(request: Observable<VillainById>): Observable<Villain>; }
ermm, I never thought of returning union types. I kind of like it :-) Me as a developer can decide to return what I want - but this means it is slightly less typed ah ah. I mean it isn't specifically typed to one return type but to 3 (union) - but I think this works.
Then the client typescript file should look like this:
export interface HeroServiceClient { findOneHero(request: HeroById): Observable<Hero>; findOneVillain(request: VillainById): Observable<Villain>; findManyVillain(request: Observable<VillainById>): Observable<Villain>; }
I must admit I have never tested it, but are you saying that the client can only receive Observables? Not sure here, the client I think can at least receive standard objects or observables - ?? Maybe I am wrong.
I don't really tend to use Promises anyway, but it kind of make sense I think as the information isn't sent over the wire until the promise completes on the server.
I think when
nestJs=true
we need to create those two differentserver
andclient
interfaces
so that it matches thenestjs
way of how to integrate this.What is your opinion on this?
I think it would work but implementing 2 x interfaces, 1 for client and 1 for server!
I like it, what does @stephenh think?
from ts-proto.
ermm, I never thought of returning union types. I kind of like it :-) Me as a developer can decide to return what I want - but this means it is slightly less typed ah ah. I mean it isn't specifically typed to one return type but to 3 (union) - but I think this works.
I think we have to do a 3 (union) type here cause that we model nestjs
correctly. If we only accept a promise
or observable
we're limiting the developer.
I must admit I have never tested it, but are you saying that the client can only receive Observables? Not sure here, the client I think can at least receive standard objects or observables - ?? Maybe I am wrong.
Yes the way nestjs
builds the client
from the .proto
file it always returns the response in an Observable
. So it doesn't matter if the data was sent via observable
or promise
. But the client wraps the response in an observable
and then returns this. I've tested this.
update
- I did some more refactoring and I think it's very close for a release. I added an
nestjs
section to thereadme
file: https://github.com/debugged-software/ts-proto#nestjs - It generated two interfaces, 1 for the client and 1 for the server. (only when
nestjs=true
) - I also added a naming convention for the interfaces in
nestjs
. - I created two dedicated functions for creating the server interface and the client interface. This way we can easily change the way how we build the interfaces for
nestjs
. (generateNestjsService
andgenerateNestjsServiceClient
).
@stephenh @iangregsondev
Please take a look at the readme
and let me know if there are some changes needed for a release. https://github.com/debugged-software/ts-proto#nestjs
from ts-proto.
update 2
- when
nestjs=true
it also generates anclass decorator
for exampleHeroServiceControllerMethods
(based on the name of the service). Normally innestjs
you would need to add@GrpcMethod('SERVICE_NAME')
or@GrpcStreamMethod('SERVICE_NAME')
. This however would fail in runtime when you accidentally change your service name. But because we generate thisclass decorator
for you, you won't have this risk anymore. The only thing you need to do is provide thisdecorator
on thecontroller
class. The decorator will take care of applying the@GrpcMethod
and@GrpcStreamMethod
@Controller('hero')
// Generated decorator that applies all the @GrpcMethod and @GrpcStreamMethod to the right methods
@HeroServiceControllerMethods()
export class HeroController implements HeroServiceController {
...
}
- based on the
.proto
we'll generate aconst
for exampleHERO_PACKAGE_NAME
andHERO_SERVICE_NAME
this way your code breaks if you change your package or service name. (It's safer to have compiler errors than runtime errors)
from ts-proto.
@iangregsondev It didn't but I've implemented a check for this and now it expects a type void
when an Empty
return is set for the response
.
from ts-proto.
Released in v1.20.0. Thanks @toonvanstrijp
from ts-proto.
Related Issues (20)
- I'm using the buf tool to generate a ts file and have set `-ts_proto_opt=esModuleInterop=true`, but the generated code still contains `import Long = require("long");` HOT 1
- Module has already exported a member named 'ServerStreamingMethodResult' HOT 7
- basic_string::_M_construct null not valid HOT 5
- removeEnumPrefix not working HOT 1
- fromJSON should accept both snake_case and camelCase keys HOT 1
- Provide Options type for execution in TS HOT 1
- Add common types to their own file/export w/outputIndex option HOT 2
- env=browser treated same as env=both. Causes globalThis.Buffer typescript error HOT 5
- Mistake in nestjs doc: `HeroesService` should be `HeroServiceClient`? HOT 1
- Allow NestJS to leverage Generic definitions HOT 2
- Don't encode proto2 optional fields HOT 7
- `outputSchema=true` throwing when compiling `googleapis` protos HOT 1
- Why every message is genereated as optional HOT 2
- Value/ListValue do not include the required dependencies when using nestJs=true
- Error message type naming collision HOT 5
- Optionally sort keys in output interface types? HOT 2
- Should deprecated fields be optional? HOT 3
- How to use with node js HOT 1
- Wrapper Types conversion does not work for repeated field
- Oneof objects do not follow generated interface. HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from ts-proto.