Generate both grpc-node and grpc-web simultaneously

[Multiply]

Would it make sense to be able to generate both grpc-node and grpc-web services at the same time, rather than the current if/else we have today?

[jonnyreeves]

Do you have this use case?

…

On Tue, 5 Nov 2019, 22:15 Jens Ulrich Hjuler Fosgerau, < ***@***.***> wrote: Would it make sense to be able to generate both grpc-node and grpc-web services at the same time, rather than the current if/else we have today? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#211?email_source=notifications&email_token=AABQ36N7QFT3YCJZE5HFBDLQSHWBDA5CNFSM4JJLBNX2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HXBKJHA>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABQ36NMIE3KQ5VJ2YBMLWLQSHWBDANCNFSM4JJLBNXQ> .

[Multiply]

I would like to use the service interface from grpc-node to build the GRPC Server, and would like to use the client from grpc-node to implement server-side clients.

And in turn use grpc-web client to implement clients that run in the browser.

Alternatively, I would like to optionally generate either the base d.ts files, or one of the services, to only generate a single set of files in one protoc call, or all of them in one go.

[jonnyreeves]

That makes sense. Do you have thoughts on how this could work from a user perspective?

…

On Wed, 6 Nov 2019 at 07:46, Jens Ulrich Hjuler Fosgerau < ***@***.***> wrote: I would like to use the service interface from grpc-node to build the GRPC Server, and would like to use the client from grpc-node to implement server-side clients. And in turn use grpc-web client to implement clients that run in the browser. Alternatively, I would like to optionally generate either the base d.ts files, or one of the services, to only generate a single set of files in one protoc call, or all of them in one go. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#211?email_source=notifications&email_token=AABQ36J6TDR6DXYMZSVY3KDQSJY4XA5CNFSM4JJLBNX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDFTEUQ#issuecomment-550187602>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABQ36OUTX3RQ4ELJRG5ZXTQSJY4XANCNFSM4JJLBNXQ> .

[Multiply]

Scenario 1: Generate everything in one call
Change the service parameter to allow comma separated values: https://github.com/improbable-eng/ts-protoc-gen/blob/master/src/index.ts#L30-L31
Change the else if to an if: https://github.com/improbable-eng/ts-protoc-gen/blob/master/src/index.ts#L49-L55

Scenario 2: Generate only one set of files per call
Add another parameter generate (or some other meaningful name that is not service), and allow comma separated values like in the proposal above.
Add an if around the base typescript files: https://github.com/improbable-eng/ts-protoc-gen/blob/master/src/index.ts#L44-L47
When generate parameter contains base, output the base typescript files we just added an if around.
When generate parameter contains grpc-web, output the service web typescript files.
When generate parameter contains grpc-node, output the service node typescript files.

---

The latter proposal can be backwards compatible, and will support both scenarios. All in one go, or one set of files at a time.

Edit: I don't know the codebase too well, so this might not be how any of this is intended to work at all. Just an idea I had.

[jonnyreeves]

Awesome, thank you for articulating these so clearly :) Do you have a preference for either option? My personal preference would be for the first option, but I'm open to persuasion.

The latter proposal can be backwards compatible, and will support both

scenarios. All in one go, or one set of files at a time. Do you think that Option 1 is not backwards compatible?

…

On Wed, 6 Nov 2019 at 10:40, Jens Ulrich Hjuler Fosgerau < ***@***.***> wrote: *Scenario 1: Generate everything in one call* Change the service parameter to allow comma separated values: https://github.com/improbable-eng/ts-protoc-gen/blob/master/src/index.ts#L30-L31 Change the else if to an if: https://github.com/improbable-eng/ts-protoc-gen/blob/master/src/index.ts#L49-L55 *Scenario 2: Generate only one set of files per call* Add another parameter generate (or some other meaningful name that is not service), and allow comma separated values like in the proposal above. Add an if around the base typescript files: https://github.com/improbable-eng/ts-protoc-gen/blob/master/src/index.ts#L44-L47 When generate parameter contains base, output the base typescript files we just added an if around. When generate parameter contains grpc-web, output the service web typescript files. When generate parameter contains grpc-node, output the service node typescript files. ------------------------------ The latter proposal can be backwards compatible, and will support both scenarios. All in one go, or one set of files at a time. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#211?email_source=notifications&email_token=AABQ36OBXFFVUGH2CGNL2GTQSKNIVA5CNFSM4JJLBNX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDGC4YY#issuecomment-550252131>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABQ36MZG5Y4G5NBKTIBGYDQSKNIVANCNFSM4JJLBNXQ> .

[Multiply]

Scenario 1 is definitely backwards compatible, but it does not allow you to skip the generation of the base fileset. So if I were to run two protoc on the same directory, one for grpc-web, and one for grpc-node, it would cause the base typescript files to be generated twice, unless we go for an approach like Scenario 2.

Edit: Just to clarify my initial backwards compatible comment; It was meant as if we reuse the service parameter, we won't be backwards compatible, if we were to wrap the base typescript files in an if on the same parameter.

[jonnyreeves]

Hey Jens,

Scenario 1 is definitely backwards compatible, but it does not allow you

to skip the generation of the base fileset. So if I were to run two protoc on the same directory, one for grpc-web, and one for grpc-node, it would cause the base typescript files to be generated twice, But if you are able to specify multiple 'service' outputs (ie: `service=grpc-web,grpc-node') then there would be no need to run protoc twice? Am I missing something?

…

On Wed, 6 Nov 2019 at 11:49, Jens Ulrich Hjuler Fosgerau < ***@***.***> wrote: Scenario 1 is definitely backwards compatible, but it does not allow you to skip the generation of the base fileset. So if I were to run two protoc on the same directory, one for grpc-web, and one for grpc-node, it would cause the base typescript files to be generated twice, unless we go for an approach like Scenario 2 — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#211?email_source=notifications&email_token=AABQ36MWL34YJKG7XJ3DIY3QSKVL3A5CNFSM4JJLBNX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDGISSA#issuecomment-550275400>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABQ36JUYKSRU3OLBLJ7KK3QSKVL3ANCNFSM4JJLBNXQ> .

[Multiply]

@jonnyreeves that's correct.

The 'need' for Scenario 2 stems from Dig-Doug/rules_typescript_proto#10 (comment) where I'd like to avoid generating the base ts files twice.

I can work around it, though. Was just simple to implement (in a backwards-compatible way) now that I'm already looking at implementing one of the scenarios above.

Let me know what you think, and I can try to submit a PR tomorrow.

[jonnyreeves]

Ah great, thanks for sharing. PRs welcome, happy for you to take the lead on the design. Let me know if I can help 👍

…

On Wed, 6 Nov 2019, 19:01 Jens Ulrich Hjuler Fosgerau, < ***@***.***> wrote: @jonnyreeves <https://github.com/jonnyreeves> that's correct. The 'need' for Scenario 2 stems from Dig-Doug/rules_typescript_proto#10 (comment) <Dig-Doug/rules_typescript_proto#10 (comment)> where I'd like to avoid generating the base ts files twice. I can work around it, though. Was just simple to implement (in a backwards-compatible way) now that I'm already look at implementing one of the scenarios above. Let me know what you think, and I can try to submit a PR tomorrow. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#211?email_source=notifications&email_token=AABQ36M24TSH5KQP5R4GRN3QSMICJA5CNFSM4JJLBNX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDHUCEY#issuecomment-550453523>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABQ36IVM6R3VDFKPC4DGD3QSMICJANCNFSM4JJLBNXQ> .

[Multiply]

@jonnyreeves I ended up going with scenario 2. I've made a WIP PR here: #212

Please let me know of any changes or missing steps I need to complete, for it to be in a mergeable state.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.