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

[Feature Request] Add option to choose which fields should be added to toJson and fromJson #1244

Closed
joeldomke opened this issue Nov 14, 2022 · 2 comments · Fixed by #1256
Closed

Comments

@joeldomke
Copy link

This is a more structured approach to #1178 I already started with the implementation and have written the necessary tests, but there are still some open questions, which I listed below. Once those are answered, I should be able to finish the implementation of this feature.

Problem

The current implementation does not give a lot of flexibility when it comes to defining which properties should be included in fromJson and toJson.

A property will be included in fromJson if all of the following are true:

  • JsonKey.ignore == false
  • JsonSerializable.createFactory == true
  • The property has a setter and getter

A property will be included in toJson if one of the cases below is true:

  • Case 1:
    • JsonKey.ignore == false
    • JsonSerializable.createToJson == true
    • JsonSerializable.createFactory == true
    • The property has a setter and getter
  • Case 2:
    • JsonKey.ignore == false
    • JsonSerializable.createToJson == true
    • JsonSerializable.createFactory == false
    • The property has a getter

Proposal

Add JsonKey.includeWith, which is of type enum IncludeWith{none, fromJson, toJson, both}. If JsonKey.includeWith is null, we resort to the previously described behavior. Otherwise, the decisions will be made as described below. This way, we ensure backwards compatibility.

A property will be included in fromJson if all of the following are true:

  • JsonKey.includeWith is one of {fromJson, both}
  • JsonSerializable.createFactory == true
  • The property has a setter

A property will be include in toJson if all of the following are true:

  • JsonKey.includeWith is one of {toJson, both}
  • JsonSerializable.createToJson == true
  • The property has a getter

Open Questions

  • Currently, if the field map is generated, it will include all elements that are part of fromJson and toJson. I am not sure what this field map is used for or whether it would be a breaking change, if we just included all properties that are either part of fromJson or toJson.
  • What should be done if both ignore and includeWith are non null?
  • What should be done if both the setter and getter have a JsonKey annotation?
  • What should be done if @JsonKey(includeWith: IncludeWith.toJson) is used on a setter, that has no corresponding getter and vice versa?
  • What should be done if @JsonKey(includeWith: IncludeWith.both) is used on a field that only has a setter or getter?
@kevmoo
Copy link
Collaborator

kevmoo commented Nov 17, 2022

Duplicate of #569

Trying to align all of these requests into one place!

@kevmoo kevmoo closed this as completed Nov 17, 2022
@kevmoo
Copy link
Collaborator

kevmoo commented Nov 22, 2022

I have a (draft) PR out on this. I'd LOVE (early) feedback about if this works for what folks need here: #1256

I still need to do some documentation and testing...

kevmoo added a commit that referenced this issue Nov 22, 2022
Deprecate JsonKey.ignore

Fixes #24
Fixes #274
Fixes #537
Fixes #569
Fixes #797
Fixes #1100
Fixes #1244
kevmoo added a commit that referenced this issue Nov 22, 2022
Deprecate JsonKey.ignore

Fixes #24
Fixes #274
Fixes #537
Fixes #569
Fixes #797
Fixes #1100
Fixes #1244
kevmoo added a commit that referenced this issue Nov 30, 2022
… serialization (#1256)

Deprecate `JsonKey.ignore`

Fixes #24
Fixes #274
Fixes #537
Fixes #569
Fixes #797
Fixes #1100
Fixes #1244
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants