-
Notifications
You must be signed in to change notification settings - Fork 62
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
chore(amplify-codegen): dart api model .fromJson() refactor #593
chore(amplify-codegen): dart api model .fromJson() refactor #593
Conversation
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## main #593 +/- ##
========================================
Coverage 85.69% 85.69%
========================================
Files 148 153 +5
Lines 7380 7480 +100
Branches 1962 1976 +14
========================================
+ Hits 6324 6410 +86
- Misses 959 972 +13
- Partials 97 98 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
a88e94f
to
702754b
Compare
702754b
to
54670ba
Compare
….fromJson() Allows decoding of non-transformed AppSync responses aws-amplify/amplify-flutter#816
54670ba
to
f337000
Compare
indent(`.where((e) => e != null)`, 4), | ||
indent(`.map((e) => ${this.getNativeType({ ...field, isList: false })}.fromJson(new Map<String, dynamic>.from(e)))`, 4), | ||
indent(`.toList()`, 4), | ||
indent(`: (json['${varName}'] as List)`), |
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.
Q: Can we be sure that json['${varName}']
is a List at this point?
The existing logic only casts to a List after checking that the type is List. The new logic casts to a List after checking that it is not null and not a Map. If json['${varName}']
were ever something other than Map, List, or null this would throw an exception.
The logic in the PR is:
- If map, handle map
- Otherwise, if not null, cast to List and handle list
- Otherwise return null
Would it make more sense to have:
- If map, handle map
- Otherwise, if List, handle list
- Otherwise, return null
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.
Good call out. I adjusted the logic to prefer type checks over null checks.
Description of changes
Flatens the nested data structure of
serializedData
found in the.fromJson()
method on codegen models. This PR is to be paired with changes in amplify-flutter#4665Issue #, if available
aws-amplify/amplify-flutter#816
Description of how you validated changes
Change were validated via:
Checklist
yarn test
passesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.