search

evestera / json_typegen

Tools and libraries to create types for Rust, Kotlin, TypeScript and Python from JSON samples
https://typegen.vestera.as
Apache License 2.0
273 stars 26 forks source link

Added option to create an alias instead of panicking when encountering a preexisting type #19

Closed AloeareV closed 4 years ago

AloeareV commented 4 years ago

This is functionality that I need locally, and perhaps it would be useful for others. I'm happy to refactor if needed, I'm not familiar enough with the package to know where the best place to put the new code is. Fixes #19

evestera commented 4 years ago

Hi, and thanks for the issue and PR.

I'm actually wondering if this makes more sense as the default behaviour than the current one, if it is not too much of a breaking change (I'll have to think a bit about that part). However, if I was to make this change it should probably be the same for all output languages, and be on the code generation level. Fortunately, Rust, Kotlin and TypeScript all support type aliases, so it should be OK to make this change for all of them.

By the way, out of curiosity, which interface are you using? Type provider, CLI, WASM or web?

AloeareV commented 4 years ago

I'm using the type provider. I'm making a whole bunch of rpc calls, writing down the responses I get, and pointing json_typegen at them so I can have concrete types instead of needing to do a bunch of runtime json parsing.

AloeareV commented 4 years ago

I don't think it would be a breaking change, I think the only thing it would change is behavior that's currently a panic. Looks like Kotlan says typealias instead of type so I'll need to at least add a check for that to not just turn a panic into a worse panic.

zancas commented 4 years ago

I'm using the type provider. I'm making a whole bunch of rpc calls, writing down the responses I get, and pointing json_typegen at them so I can have concrete types instead of needing to do a bunch of runtime json parsing.

Which means we can do compile time analyses to confirm correctness! Though... we might have run into an interesting case in spite of this.

evestera commented 4 years ago

Sorry. This took a bit longer than expected. Ended up coming to the conclusion that this should be the default, and that there would not be much point in adding an option to turn it off. There also turned out to be a few details that made it so I couldn't really use your code directly either, so ended up doing it as a new commit (51c69128f) and added you as a co-author to that one (as your code was the starting point for it).

Anyway, thanks again for the issue and the PR. I'll make a release after a couple of other small changes.

One thing I have realized might be an issue for you, based on how you describe your use though: If the type is a list, the requested type name will be used for the elements of the list, rather than the list itself.