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

snake_case to camelCase conversion #254

Open
Dugnom opened this issue Feb 23, 2024 · 7 comments
Open

snake_case to camelCase conversion #254

Dugnom opened this issue Feb 23, 2024 · 7 comments
Labels
documentation Improvements or additions to documentation

Comments

@Dugnom
Copy link

Dugnom commented Feb 23, 2024

I have set up a nest.js instance with their grpc microservice.
The grpc microservice is providing the interfaces in json default camelCase as seems to be the expected standard by grpc clients.
Is it possible for protoc-gen-ts to generate the interface to match this provided interface?
Currently the case is kept as snake_case, as it is standard in proto files.

There would be a workaround by implementing sth. equivalent to here: nestjs/nest#9047, but I'm not a fan, since the deviation in the standard seems to be from this plugin.

Is there an option to provide the typescript interfaces in camelCase?

I also don't understand how I am the only one with this problem, should I be doing something different?

@graup
Copy link

graup commented Feb 29, 2024

Have you tried --ts_opt=json_names?

@Dugnom
Copy link
Author

Dugnom commented Mar 1, 2024

I have not, because I understood the flag to allow optional json names to be defined in square brackets, like discussed here #142

I should have tried using the flag, you're right, but I was expecting the default behaviour to be the one defined in the standard, where the variable names are changed from snake to camel case.

I will not be able to test it in the next two weeks, since I've changed my workflow to use https://github.com/stephenh/ts-proto , which worked as expected without any flags and I now have other priorities. Still interested in working it out though.

Do you know if it would be the correct flag or are you just guessing by the name? Either way I appreciate you trying to help me!
If you do not know if it would work I will try to keep "testing --ts_opt=json_names" on my todo list.

@Dugnom
Copy link
Author

Dugnom commented Mar 1, 2024

I was more succesful today digging into this, then last week and found the following threads on this topic:
#78 , #89 and #98 where the consensus seemed to be to make the conversion default, but it seems like in the end nobody took the time to write any tests which stopped it from being merged.

But, as is only implied in the title of the PR #142 , the json names option was added and using it would lead to the behaviour I've expected. As I said, I was confused by the discussion about the optional json names.

This is how I understand the current situation and the timeline, maybe I am wrong, but wanted to share hoping I am helping someone else understand.
I will just use the other project instead of using barely documented workarounds in this one.

Please correct me if I misunderstood!

@thesayyn
Copy link
Owner

thesayyn commented Mar 1, 2024

This is supported by default on main branch of this repo. I have rewritten everything from scratch and now we support fromJSON and toJSON.

@Dugnom
Copy link
Author

Dugnom commented Mar 1, 2024

Thank you for the time of day you're giving me. As you can probably guess, I'm just getting into grpc communication and don't know much. I am assuming I am on main branch since I've installed the latest npm package.

I don't understand why you have implemented it like you did, but that is clearly a lack of knowledge on my part. I am sure I could have looked deeper into the documentation and made myself more familiar with the tool.

But if you forgive me reiterating the same point in every message, why is it implemented in the way it is. Is it meant not to automatically translate from snake to camel or did I just use the tool in an unintended way? I just want to understand the landscape better.

You're of course not obligated to teach me, and it is clearly a useful tool for many.
Feel free to close this issue either way and have a good time!

@thesayyn
Copy link
Owner

thesayyn commented Mar 1, 2024

No worries, it's design choice i have made when i started the plugin, original generator for the JS has converted field names to Getters/Setters which many people didn't like.

That said, --json_names is no longer applicable as we now fully support JSON payloads, if you use the Rust implementation of this plugin. you can simply do message.fromJson and message.toJson.

See #255

@thesayyn thesayyn added the documentation Improvements or additions to documentation label Mar 1, 2024
@thesayyn
Copy link
Owner

thesayyn commented Mar 1, 2024

I am going to label this as documentation issue, which it clearly is. Feel free to send a PR improving the documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants