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

Serializing enums explicitly references types unless nullable (issue … #1146

Merged
merged 4 commits into from Jun 25, 2022

Conversation

ssabdb
Copy link
Contributor

@ssabdb ssabdb commented May 9, 2022

#1145 I've fixed the issue for my own purposes by inspecting the targetType - if not nullable, I use the ! operator to remove the nullable type outputted by directly indexing _$EnumTypeEnumMap.

This includes a unit test which fails prior to the fix.

I considered doing something similar to introducing a helper function used in the enum deserialization ($enumDecode), but this just seemed simpler - since the map _$EnumTypeEnumMap is generated from the possible values of the enum, it should never result in a null, unless the variable storing the enum itself is nullable - and if that happens I don't emit an ! operator.

It does change a number of other generated files though, which might be a concern.

resolves #1145

@kevmoo
Copy link
Collaborator

kevmoo commented May 24, 2022

@ssabdb – can you look at the failures?

@ssabdb
Copy link
Contributor Author

ssabdb commented May 24, 2022

Looks like the test builds some example files which are diff'd and have changed to include the null assertion “bang” operator as an intended result of this PR.

They will need to be regenerated so the test passes - I'll figure it out today or tomorrow.

@ssabdb
Copy link
Contributor Author

ssabdb commented May 24, 2022

@kevmoo Done, I think. Pending maintainer approval to run the workflow. Apologies I missed the notification this had happened a few weeks ago.

@ssabdb
Copy link
Contributor Author

ssabdb commented May 24, 2022

@kevmoo this failure looks related to the fix you're working on in #1117?

Let me know if I need to do anything else.

@kevmoo
Copy link
Collaborator

kevmoo commented May 24, 2022

@ssabdb – the markdown failure is unrelated – I should likely get rid of that.

Copy link
Collaborator

@kevmoo kevmoo left a comment

Choose a reason for hiding this comment

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

Please add an entry to the changelog explaining this change, too!

@ssabdb ssabdb force-pushed the master branch 2 times, most recently from 81a125f to ff128db Compare May 31, 2022 11:25
@ssabdb
Copy link
Contributor Author

ssabdb commented May 31, 2022

@kevmoo. Done. I also added a second unit test to test the branching logic was working properly for nullable enums, eg. in a List type.

Before this change, serializing non-nullable enums in a subtype was incorrectly producing nullable string types.

For example, serializing a class member of type Map<enum, String> produced a type Map<String?, dynamic> where Map<String, dynamic> should have been produced.

Nullable enums should still produce nullable types, eg. serializing List<enum?> should produce List<String?>

fixes 1145
@kevmoo
Copy link
Collaborator

kevmoo commented Jun 1, 2022

Still need a changelog entry then I'll run CI and land! Thanks so much!

@ssabdb
Copy link
Contributor Author

ssabdb commented Jun 7, 2022

No worries, I'd misunderstood changelog (I thought you were talking about the gitlog!)

I'm on the road at the moment until late next week but will have a shot at getting round to it.

@ssabdb ssabdb requested a review from kevmoo June 19, 2022 23:20
kevmoo
kevmoo previously requested changes Jun 23, 2022
json_serializable/CHANGELOG.md Outdated Show resolved Hide resolved
Before this change, serializing non-nullable enums in a subtype was incorrectly producing nullable string types.

For example, serializing a class member of type Map<enum, String> produced a type Map<String?, dynamic> where Map<String, dynamic> should have been produced.

Nullable enums should still produce nullable types, eg. serializing List<enum?> should produce List<String?>

fixes 1145
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.

Enum serialization creates nullable type for non-nullable enums
2 participants