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

[TS types] can we have camel casing for field names? #40

Open
kklas opened this issue Aug 31, 2020 · 6 comments · May be fixed by #41
Open

[TS types] can we have camel casing for field names? #40

kklas opened this issue Aug 31, 2020 · 6 comments · May be fixed by #41

Comments

@kklas
Copy link
Contributor

kklas commented Aug 31, 2020

Currently it's snake_cased: https://github.com/apex/rpc/blob/master/generators/tstypes/testdata/todo_types.ts#L4

I'm using prettier on my codebase which is a very opinionated code formatter and I guess many other people are too considering the number of stars it has on github.

Prettier is strict about using camelCasing for object field names so it gives me very annoying eslint errors when I try to instantiate api types.

It would be great if we had a flag in the generator to generate camelCased fields instead. WDYT?

@tj
Copy link
Member

tj commented Sep 1, 2020

somehow I missed that haha, yeah I think we should use whatever is idiomatic for the target language, so SGTM, I'll fix that today

@tj
Copy link
Member

tj commented Sep 1, 2020

370c365 thanks!

@tj tj closed this as completed Sep 1, 2020
@tj
Copy link
Member

tj commented Sep 1, 2020

oh actually I just remembered why it's this way, because TS will serialize the fields incorrectly. If fields are defined as project_id in the schema, then you get projectId since — as far as I know — there's no easy way to map JSON property names in TS like there is in Go for example.

@tj tj reopened this Sep 1, 2020
@kklas
Copy link
Contributor Author

kklas commented Sep 1, 2020

because TS will serialize the fields incorrectly. If fields are defined as project_id in the schema, then you get projectId

Not sure what you mean here. Looked at the ts client code and this is what I understand is happening:

  • canonically we use snake_case in the schema
  • we JSON encode field names to snake_case server side
  • ts client receives and parses the objects as is
  • types match the snake_case fields in the underlying object so everything works at run time

there's no easy way to map JSON property names in TS like there is in Go for example

Yea I guess we can't do this within JSON.parse reviver but shouldn't it be straightforward to recurse on the object fields and create a new object with camel case?

@tj
Copy link
Member

tj commented Sep 1, 2020

It's definitely doable to recurse, but I'm not sure it's really worth the pain, I don't mind the snake-case much personally but I don't use any linters. If there is a clean way to do it in TS I'm definitely cool with it, but keep in mind it has to work for regular JS as well as TS.

I think we can maybe use the JSON.stringify/parse callbacks, looks like we'd have to return undefined in stringify so it doesn't add the value, and then this[camelCased] = value.

@kklas
Copy link
Contributor Author

kklas commented Sep 2, 2020

Played a bit with JSON.parse callback but couldn't get it to work on firefox:

function camelize(str) {
    capitalize = (str) => {str.charAt(0).toUpperCase() + str.slice(1)}
    tok = str.split("_")
    ret = tok[0]
    tok.slice(1).forEach((t) => ret += capitalize(t))
    return ret
}

JSON.parse(o, (key, value) => {
  this[camelize(key)] = value
  delete(this[key])
})

I guess if you return undefined it just throws the whole thing away? Maybe I missed something.

This seems a bit hacky anyways and I guess it would also depend on JSON.parse/stringify implementation in the engine so I wouldn't go for this approach.

I'll do a PR so let's see how it can be implemented.

@kklas kklas linked a pull request Sep 2, 2020 that will close this issue
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 a pull request may close this issue.

2 participants