Skip to content
This repository has been archived by the owner on Jun 15, 2020. It is now read-only.

Maps are not generated properly #6

Open
michelgb opened this issue Feb 6, 2016 · 6 comments
Open

Maps are not generated properly #6

michelgb opened this issue Feb 6, 2016 · 6 comments

Comments

@michelgb
Copy link
Contributor

michelgb commented Feb 6, 2016

Hello. I notice maps are not properly converted to typescript definitions. For example.

map<string, int32> attr1 = 1;

should convert to

attr1 { [key: string]: number }

@DouglasHeriot
Copy link
Contributor

I need this too. Also seems "repeated" and "oneof" fields aren’t generated properly either.
About to have a look at how difficult it’ll be to implement…

@DouglasHeriot
Copy link
Contributor

It seems repeated fields are fine, but oneofs and maps are missing. These are both protobuf 3 features.

This is the first time I’ve ever used Dusty templates. I don’t fully see their value in this case – I’ve got to look at how to split the logic between the command.ts and the templates. I’m pretty tempted to just remove Dusty and put it all in Typescript.

@fungiboletus
Copy link
Member

Hei,
Yes I don't think I implemented oneofs and maps. You could remove the Dusty templates, if you think the code will be more beautiful without them. The idea was to avoid a lot of ugly string concatenations in the source code.

@DouglasHeriot
Copy link
Contributor

Done, works for me. Not tested extensively with different types of keys.

I ended up keeping Dusty, once I found how to add simply if statements. Just not sure how much repetition it’ll lead to, if maps are implemented in get/set methods.

@DouglasHeriot
Copy link
Contributor

Well, I should have looked earlier – there’s a fork that already has support for proto3 maps: https://github.com/michelgb/Proto2TypeScript/blob/master/templates/interface.dust

It’s very similar to my implementation, but more complete. Might need to have a look at it and consider merging it instead.

@DouglasHeriot
Copy link
Contributor

This can be closed now that #8 is merged.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants