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

Nullable types in golang #1418

Closed
wiegell opened this issue Jun 22, 2023 · 10 comments
Closed

Nullable types in golang #1418

wiegell opened this issue Jun 22, 2023 · 10 comments
Labels
enhancement New feature or request released on @next

Comments

@wiegell
Copy link

wiegell commented Jun 22, 2023

Reason/Context

Please try answering few of those questions

  • Why we need this improvement?
    Nullable strings are vital in many languages, also in go! In our current project we have put a lot of effort to support nullability between frontend and bff (graphql) - if it cannot be kept between services (asyncapi), it's a dealbreaker!
    Currently type: ["integer", "null"] is translated to interface{} which is not typesafe. Other generators, e.g. https://github.com/99designs/gqlgen solves the problem by converting optional primitives to pointers, which are nullable. E.g. a type string becomes type *string.

53edd32
This change addresses nullable types in typescript - could it be expanded to go?

Description

Please try answering few of those questions

  • Will this be a breaking change?
    Yes
@wiegell wiegell added the enhancement New feature or request label Jun 22, 2023
@github-actions
Copy link
Contributor

Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@jonaslagoni
Copy link
Member

@wiegell thanks for opening the issue!

As I am not that well versed in Go, hope you don't mind answering some questions 😅 Is it ALL types that just need *type for it to be nullable?

v2 actually can handle this, we just need to alter a few lines of code for the type generation 🙂

@wiegell
Copy link
Author

wiegell commented Jun 25, 2023

Yes all pointers (*) are nullable, but some value types can also be nil.

See this proposal: golang/go#30177

Motivation -> 2nd bullet explains using a pointer to simulate nullable.
Proposal -> first bullet list is a list of types that cannot be nil.

So it would be needed to add * to the golang type corresponding to a nullable version of the following asyncapi types:
string
number
integer
boolean
object

I presume asyncapi array type is translated to golang slice (which is nullable in contrast to a golang array of fixed length), so in that case a value type without * will suffice.

I'm not with my computer the next couple of days, but i would like to check if my reasoning here checks out with the graphql generator mentioned above, and i'll get back to you!

@zekth
Copy link

zekth commented Jul 4, 2023

As a really good example you can check this project for struct generation: https://github.com/deepmap/oapi-codegen

But we also need to take care of non required with the omitempty struct tag: https://pkg.go.dev/encoding/json#Marshal

@wiegell
Copy link
Author

wiegell commented Jul 17, 2023

So i just checked with openapi - they use string pointers for non-required fields as well and then value types when the string is required.

@arthurspa
Copy link

I started using AsyncAPI generation for Go and found this current limitation, which is a deal breaker for me as well.

@jonaslagoni
Copy link
Member

@arthurspa @wiegell @zekth

In Modelina v2, the constraint logic now have access to nullable and non-required fields, so all we have to change is: https://github.com/asyncapi/modelina/blob/master/src/generators/go/GoConstrainer.ts

You can use the Java constrainer as reference:

if (constrainedModel.options.isNullable || !partOfProperty?.required) {

Happy to help guide as much as needed 🙂

@arthurspa
Copy link

Thanks @jonaslagoni for pointing out the file. I'll take a look at that some time this week.

@arthurspa
Copy link

Just an update: I ended up writing my own template and made a quick fix for the nullable types I needed, which is not worth sharing since it's more a hack than a proper solution. So I won't spend time with this ticket, unfortunately.

@asyncapi-bot
Copy link
Contributor

🎉 This issue has been resolved in version 4.0.0-next.31 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released on @next
Projects
None yet
Development

No branches or pull requests

5 participants