Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement option to generate alternative typescript format #25

Merged
merged 1 commit into from
May 21, 2024

Conversation

dpup
Copy link
Owner

@dpup dpup commented May 20, 2024

Hello @seanami,

This turned out to be a pretty big change.

The primary change is to introduce a use_static_classes flag which switches
the output format. With the flag set to false, instead of using a static class
a function is generated along with a stateful Client class. See #23

To support this change I had to rework how integration tests work. Within
/test/integration there are now sub-folders that represent different
configurations of the generator or gRPC gateway. I've also removed Karma — which
was deprecated — and switched to Web Test Runner, which looks a lot simpler.

I've also got rid of the /scripts/ directory and consolidated all the commands
within the Makefile.

I've also made it so none of the generated files for the integration tests are checked
into source control.

Please review the following commits I made in branch dan/new-format:

71ec29f (2024-05-20 16:11:32 -0700)
Implement option to generate alternative typescript format

Code review reminders, by giving a LGTM you attest that:

  • Commits are adequately tested
  • Code is easy to understand and conforms to style guides
  • Incomplete code is marked with TODOs
  • Code is suitably instrumented with logging and metrics

@dpup dpup requested a review from seanami May 20, 2024 23:14
@dpup dpup force-pushed the dan/new-format branch 2 times, most recently from a132d46 to f55a1f7 Compare May 21, 2024 01:02
@dpup dpup force-pushed the dan/new-format branch from f55a1f7 to 77664a4 Compare May 21, 2024 16:21
@dpup dpup merged commit a042c37 into main May 21, 2024
2 checks passed
@dpup dpup deleted the dan/new-format branch May 21, 2024 19:55
Copy link
Collaborator

@seanami seanami left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Nice, this looks great!

When `use_static_classes` is set to false, you can implement your code will look like this:

```ts
import { increase } from "./counter.pb";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could add a third example here suggesting a way to get an equivalent to the old format, if desired:

import { * as CounterService } from "./counter.pb";

(I think this works, right?)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should work, I think you'd also get CounterService.CounterServiceClient

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