-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
[dart-dio-next] Improve handling nullable objects #10118
[dart-dio-next] Improve handling nullable objects #10118
Conversation
Not everything is nullable, we need to default to non-nullable. Do you have a sample spec for collections with nullable content? |
I created #10122 for built_value update |
Looking at Kotlin and C# implementations, none of them support null in collections. There also doesn't seem to be a representation for this in the specification. Maybe a ref with |
@kuhnroyal but there is a spec for this
|
The only problem I am facing is: |
I see, I didn't know that |
@kuhnroyal what do you think about this change? I even removed the java code for the serializer type and wrote it in mustache using recursive patterns |
also @wing328 can this be included in 5.2.1 ? |
I like it but there is still a couple things open. I don't think this should go into 5.2.1 |
@kuhnroyal is there anything else I can fix on my end? |
samples/openapi3/client/petstore/dart-dio-next/petstore_client_lib_fake/README.md
Outdated
Show resolved
Hide resolved
...es/openapi3/client/petstore/dart-dio-next/petstore_client_lib_fake/lib/src/model/animal.dart
Outdated
Show resolved
Hide resolved
...api-generator/src/main/java/org/openapitools/codegen/languages/DartDioNextClientCodegen.java
Outdated
Show resolved
Hide resolved
…to dart-dio-next/use-nullable-json-objects
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really like what you did here! 2 small changes requested
...tstore/dart-dio-next/petstore_client_lib_fake/lib/src/model/additional_properties_class.dart
Outdated
Show resolved
Hide resolved
...tstore/dart-dio-next/petstore_client_lib_fake/lib/src/model/additional_properties_class.dart
Outdated
Show resolved
Hide resolved
…to dart-dio-next/use-nullable-json-objects
...api-generator/src/main/resources/dart/libraries/dio/serialization/built_value/class.mustache
Outdated
Show resolved
Hide resolved
...api-generator/src/main/resources/dart/libraries/dio/serialization/built_value/class.mustache
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work, that will also make it easier to fix a couple problems around missing serializers. Thanks!
@agilob Do you have anything else here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@wing328 This can be merged |
Currently nullable types need to be handled better at serialization level, for example:
deserializing
BuiltMap<String,JsonObject>
when one of the values isnull
will throw an error at run time, sinceJsonObject
is not a nullable typethese changes are not perfect, especially changing these
but I am not sure what's the best approach for putting the null operator (
?
) after a type ?also this PR makes use of the
FullType.nullable
named constructor for defining serializersPR checklist
master
,5.3.x
,6.0.x
CC: @jaumard @josh-burton @amondnet @sbu-WBT @kuhnroyal @agilob