Skip to content

Conversation

chris-kruining
Copy link

I have currently implemented this as am extra setting which is a boolean.

I would not at all be suprised if the future holds more styles of code generation, so it might be worth introducing a string option called style (or something along those lines) so that more styles are more easily added :D

@chris-kruining
Copy link
Author

depends on #111

@thesayyn
Copy link
Owner

iterator methods seem really cool. the only thing that I see is not acceptable is that we depend on a non-official third-party dependency. perhaps we have some boilerplate code in our service and still match the current behavior? like we did in the experimental unary promise option?

@chris-kruining
Copy link
Author

@thesayyn I know the async is not compatible with @grpc/grpc-js. But I wanted to have a modern implementation that has just grpc support, not grpc-web (@grpc/grpc-js is a grpc-web lib).

I would love to make my @fyn-software/grpc lib an official one, any clue on how to achieve that? (besides completing the implementation)

@chris-kruining
Copy link
Author

chris-kruining commented Jan 21, 2022

another method I had thought of, but would be a boatload of extra work, is to setup a build-system-esc solution with a load of hooks, so that you inject your own style of generation. This together with the grpc_package option open up a lot op posobilities without being limited to the horribly outdated API of @grpc/grpc-js

@thesayyn
Copy link
Owner

I would love to make my @fyn-software/grpc lib an official one, any clue on how to achieve that? (besides completing the implementation)

Unfortunately, this is not up to me. your patches to the @grpc/grpc-js have to be landed on https://github.com/grpc/grpc-node. I can not force users to rely on a third-party package. that's why I proposed putting this boilerplate code to generated sources instead of using a third party package.

@thesayyn
Copy link
Owner

thesayyn commented Jan 21, 2022

another method I had thought of, but would be a boatload of extra work, is to setup a build-system-esc solution with a load of hooks, so that you inject your own style of generation

yes, this could an interesting thing though.

@chris-kruining
Copy link
Author

Unfortunately, this is not up to me. your patches to the @grpc/grpc-js have to be landed on https://github.com/grpc/grpc-node. I can not force users to rely on a third-party package. that's why I proposed putting this boilerplate code to generated sources instead of using a third party package.

I understand, I cannot imagine the folks from @grpc/grpc-node are looking for a complete rewrite. So for now I'll keep the fork around and publish the patch under my own package, I already have the protoc docker image I use working with this lib, so swapping it out should be easy.

@thesayyn
Copy link
Owner

I understand, I cannot imagine the folks from @grpc/grpc-node are looking for a complete rewrite. So for now I'll keep the fork around and publish the patch under my own package, I already have the protoc docker image I use working with this lib, so swapping it out should be easy.

perhaps grpc_package_name option that we have, might help you. but you still get type mismatch with official grpc package.

@chris-kruining
Copy link
Author

perhaps grpc_package_name option that we have, might help you. but you still get type mismatch with official grpc package.

you are right, that's why I use it like this: --ts-out grpc_package=@fyn-software/grpc,async=true,no_namespace=true.

I did consider to make it so that grpc_package=@fyn-software/grpc would generate the async interfacing code, but than no one could use/create an alternative library also works with the generators.

I also considered to generate boiler plate like with the unary promise, but I dislike the idea that my runtime should suffer for programming comfort, I wanted the best of both.

@thesayyn thesayyn force-pushed the typescript branch 3 times, most recently from 339add8 to 395b68f Compare January 21, 2022 15:29
@thesayyn thesayyn force-pushed the typescript branch 11 times, most recently from 653c250 to 4c3460d Compare January 24, 2022 11:30
@thesayyn thesayyn deleted the branch thesayyn:typescript January 24, 2022 11:48
@thesayyn thesayyn closed this Jan 24, 2022
@thesayyn
Copy link
Owner

upps.

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