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

Naming convention: snake_case vs. camelCase #34

Closed
soedirgo opened this issue Jul 14, 2020 · 7 comments · Fixed by #37
Closed

Naming convention: snake_case vs. camelCase #34

soedirgo opened this issue Jul 14, 2020 · 7 comments · Fixed by #37
Labels

Comments

@soedirgo
Copy link
Member

This is a bit of a pedantic (breaking) issue that I found while writing up the OpenAPI spec: due to the naming convention of Postgres (snake_case), most of the fields in the API are also snake_case. It'd be more idiomatic to use camelCase.

There are also other minor issues that I found (mostly to do with types, e.g. string should be number), so we can do all that plus the OpenAPI stuff in one go. Probably not gonna eliminate all of such issues, but we can at least kaizen our way there.

@kiwicopple
Copy link
Member

Good call Bobbie

Postgres (snake_case)
OpenAPI spec (camelCase)
It'd be more idiomatic to use camelCase.

Despite being REST idiomatic, I think there are some important benefits of snake_case:

  • it's idiomatic to postgres (snake_case doesn't break Open API as far as I can tell, but camelCase does break Postgres)
  • less casting - no need to cast each time between snake and camel - better code simplicity

It's easy to say to developers "our API uses snake_case", but harder to code snake<->camel just because of convention.

I think at the moment we are mixing the two (includeSystemSchemas), and this is bad. My preference would be to make everything snake_case (include_system_schemas). Let me know your thoughts

@soedirgo
Copy link
Member Author

soedirgo commented Jul 14, 2020

In general, I prefer consistent and predictable design at the cost of implementation complexity. In this case:

  • The only place we need to change snake -> camel is, I think, the SQL templates, which are well-isolated. While we can change the API to be 100% snake_case, we're still going to live with snake & camel within pg-api.

  • For inbound property names (POST, PATCH), these won't directly interact with Postgres (there are also some fields I added for convenience, like dropDefault), so it shouldn't break Postgres.

  • For outbound property names (GET, res.data), it's a bit of effort to tweak the templates, but the API becomes cleaner. On that note, I haven't seen other DB interfaces/ORMs adopting snake_case.

  • Given that we want to provide an easy-to-use interface for less-than-powerusers, I think we should prioritize having a clean API. As per the 2 points above, all interactions with the DB are intercepted, so breaking Postgres due to naming shouldn't be a concern.

  • The last one, and this one is my bad, I blindly used camelCase for most (all?) of POST and PATCH, at this point it's probably less effort to be 100% camel than 100% snake.

P.S. My biggest concern is breaking prod. How much interaction is there with pg-api at the moment? Maybe we can make it stable enough before we ship?

@kiwicopple
Copy link
Member

yeah it's already used in prod so it will definitely break. The camel -> snake conversions will be safer because we aren't using query params, but we are using response keys/objects.

at this point it's probably less effort to be 100% camel

Probably not with the changes to production but regardless, we should prioritize the effort going forward over the effort now. Writing less code is always my number 1 concern, more than blindly following a convention.

My gut is snake_case is going to save us a mountain of effort going forward, but I'll leave the final decision to you

Side note, I used to always write snake-camel converters between my PG database and my API. When I started using postgREST I stopped doing that and it actually made things conceptually simpler - camel = code, snake = database. Also reduced my LOC. This convention isn't unheard of either, but not really necessary here.

@github-actions
Copy link

🎉 This issue has been resolved in version 0.10.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@wiesson
Copy link

wiesson commented Dec 6, 2021

Side note, I used to always write snake-camel converters between my PG database and my API. When I started using postgREST I stopped doing that and it actually made things conceptually simpler - camel = code, snake = database. Also reduced my LOC. This convention isn't unheard of either, but not really necessary here.

@kiwicopple - is postgREST able to automatically convert from snake to camel and vice versa for me? I have the issue right now, we are transitioning from the firestore and everything is based on camelCase but I understand that it is a way easier within postgres to have snake case (I don't want to handle quotes etc).

// edit: Maybe this is the wrong place to ask that topic

@soedirgo
Copy link
Member Author

soedirgo commented Dec 7, 2021

@wiesson I think this issue supabase/postgrest-js#204 is more relevant.

@wiesson
Copy link

wiesson commented Dec 7, 2021

@wiesson I think this issue supabase/postgrest-js#204 is more relevant.

Oh yes, this is what I was looking for! I just hoped, postgREST could already do this :)

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

Successfully merging a pull request may close this issue.

3 participants