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

Erase domains types wrapper #80

Merged
merged 9 commits into from
Jun 9, 2022

Conversation

Virgiel
Copy link
Member

@Virgiel Virgiel commented Jun 3, 2022

Our generated domain types do not enforce the domain rules and are only used for ser/deser with the Postgres protocol. By being exposed to the client in parameters and row types, they bring noise without any benefit.

Deserialization

Deserialization is easy, we can simply skip the domain wrapper as it is not needed.

Serialization

We rely on the fact that we generate FromSql ourselves to inject the wrapper at the last minute.

Depends on #73

@Virgiel Virgiel marked this pull request as ready for review June 3, 2022 13:27
@LouisGariepy
Copy link
Member

Looks good to me! Is this ready to merge?

@Virgiel
Copy link
Member Author

Virgiel commented Jun 9, 2022

I'm not sure if the domain wrapper type should be in the cornucopia client or in the generated code. I'd like to keep it private as it's only used internally, but copying it every time into the codegen seems silly.

If you are happy with the current design, you can merge !

@LouisGariepy
Copy link
Member

LouisGariepy commented Jun 9, 2022

That is indeed a good question.

I think we can make it very clear that cornucopia_client is intended to be consumed by tthe generated code, and not something users should depend on. If there are items in cornucopia_client that are intended for users (I think ArrayIterator is one?), then we could move them to a separate cornucopia_utils crate or whatever, intended for advanced users.

What do you think?

@Virgiel
Copy link
Member Author

Virgiel commented Jun 9, 2022

Another crate may be excessive. How about a core module in the cornucopia client using doc(hidden). This way, the code can be used by our generated code but is hidden from the user. rust-analyzer could even support it in the future.

@LouisGariepy LouisGariepy merged commit 36a4a0a into cornucopia-rs:main Jun 9, 2022
@Virgiel Virgiel deleted the hidden_domain_2 branch June 23, 2022 06:08
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 this pull request may close these issues.

2 participants