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

Use map syntax to clean up null handling in toJson functions #1443

Merged
merged 7 commits into from
Aug 9, 2024

Conversation

kevmoo
Copy link
Collaborator

@kevmoo kevmoo commented Aug 8, 2024

No description provided.

@kevmoo
Copy link
Collaborator Author

kevmoo commented Aug 8, 2024

Inspired by dart-lang/sdk@6c92bab

Thanks @parlough !!

@kevmoo kevmoo requested a review from natebosch August 8, 2024 23:54
'Inigo',
'Montoya',
DateTime(1560, 5, 5),
middleName: 'Bob',
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added `middleName since it's nullable – to exercise behavior here!

return val;
}
Map<String, dynamic> _$OrderToJson(Order instance) => <String, dynamic>{
if (instance.count case final val?) 'count': val,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the flip-side (I can't comment there), it would be great for final code size to have a library of helpers for common patterns like

..count = (json['count'] as num?)?.toInt()

-->

..count = helper$.nullableInt(json, 'count')

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How could we measure the delta here? I'm game, but I'd like to see numbers...

I wonder what the lint is here, too. If this is such a big win...how can we codify it more?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we land parts-with-imports this gets easy enough to just do it without worrying about the specific delta.

https://github.com/dart-lang/language/blob/main/working/augmentation-libraries/parts_with_imports.md

IMO it's not worth spending much time one before then.

json_serializable/lib/src/constants.dart Outdated Show resolved Hide resolved
@kevmoo kevmoo merged commit 55d68f4 into master Aug 9, 2024
18 checks passed
@kevmoo kevmoo deleted the map_silly branch August 9, 2024 04:30
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.

3 participants