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

Type name collision resolution should be smarter #49

Open
chrisgarber opened this issue Nov 8, 2024 · 2 comments
Open

Type name collision resolution should be smarter #49

chrisgarber opened this issue Nov 8, 2024 · 2 comments

Comments

@chrisgarber
Copy link

Hey @pabra! Thanks so much for this great library. My company is using to great effect. The struggle we ran into is that if you have a nested object with colliding keys, the generated types aren't named very well. e.g.:

{
  "people": {
     "names": ["john", "judy"]
  },
  "animals": {
    "names": ["beau", "max"]
  }
}

would generate something like:

type Names = ["john", "judy"];
type Names2 = ["beau", "max"];

We thought it would be more powerful to look up the object chain to the parent for collisions so it would generate:

type Names = ["john", "judy"];
type AnimalNames = ["beau", "max"]; //open question here about how to combine the names, we use _, but others might use different characters

or maybe even

type PeopleNames = ["john", "judy"];
type AnimalNames = ["beau", "max"]; //open question here about how to combine the names, we use _, but others might use different characters

I have a POC in a patch locally for something like the first option. We were wondering if collision resolution was something you'd want to bake into the project differently, maybe as an option in args? Otherwise something like this would likely be a breaking change

@pabra
Copy link
Owner

pabra commented Nov 11, 2024

Hi @chrisgarber, thanks for reporting. I think that's a good idea to name Types after their hierarchical path. In your example, I would prefer

type PeopleNames = ["john", "judy"];
type AnimalsNames = ["beau", "max"];

I think, I would prefer it so much to make that the new default.

Honestly, I haven't touched this repo in years and am not sure when to find time, to look into your suggestion. Do you mind creating an PR or sharing patch/branch?

PS: I am glad, this tiny project is useful to you.

@chrisgarber
Copy link
Author

More than happy to share the patch I made. It currently works like the first example, i.e. the first key takes the short name and subsequent ones get the prefix. I'll put up what I have for now even though it's a little messy. I can look into solving the name collisions symmetrically so both collision members would get hierarchical paths as well as fix combining characters as a follow up

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

No branches or pull requests

2 participants