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

Support using non-nullable JsonConverter on nullable properties #1136

Merged
merged 7 commits into from Apr 28, 2022

Conversation

rrousselGit
Copy link
Contributor

This allows using JsonConverter<int, ...> on int? value.

This also gracefully handles null JSON when using JsonConverter<..., int> on Object? value, such that null is returned instead of throwing

fixes #822

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.

Gah! Just thought of this. Create just one function. Call it $convertOrNull. There is zero benefit to have two identical functions with swapped generic arguments – right?

@rrousselGit
Copy link
Contributor Author

Good call, I didn't realise

@rrousselGit
Copy link
Contributor Author

Actually, I'm not sure if that's a meaningful improvement

Only one of the two functions (fromJson) is doing a cast, so their body has a difference. We always do the cast, but that'd negatively impact toJson which doesn't need one.

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 update the version to 6.3.0-dev and add a changelog entry

We can see if we can land you other change here, too!

@rrousselGit
Copy link
Contributor Author

Done

@kevmoo kevmoo merged commit 09cca32 into google:master Apr 28, 2022
@rrousselGit rrousselGit deleted the non-nullable-json-converter branch April 28, 2022 17:52
@hasimyerlikaya
Copy link

Hi,

I have problem with this merge.
I use two converter for a custom type wich name is Ref.
I can't generate g.dart file because there is an error.

"Found more than one matching converter for Ref?."

I don't want to add annotation top of nullable field every time.
How can I use both converter with class level annotation?

Non-Nullable Converter

class RefResultConverter implements JsonConverter<Ref, dynamic> {
  const RefResultConverter();

  @override
  Ref fromJson(dynamic json) {
    if (json != null) {
      if (json["@ref"] != null) {
        final refResult = RefResult.fromJson(json["@ref"] as Map<String, dynamic>);
        return refResult.asRef();
      } else if (json["ref"] != null && json["ref"]["collection"] != null) {
        final collection = json["ref"]["collection"] as String;
        final id = json["id"] as String;
        return Ref(Collection(collection), id);
      }
    }
    return Ref.empty();
  }

  @override
  Ref toJson(Ref ref) {
    if (ref.id == null || ref.id == "") {
      return Ref.empty();
    }

    return ref;
  }
}

Nullable Converter

class RefNulableResultConverter implements JsonConverter<Ref?, dynamic> {
  const RefNulableResultConverter();

  @override
  Ref? fromJson(dynamic json) {
    if (json != null) {
      if (json["@ref"] != null) {
        final refResult = RefResult.fromJson(json["@ref"] as Map<String, dynamic>);
        return refResult.asRef();
      } else if (json["ref"] != null && json["ref"]["collection"] != null) {
        final collection = json["ref"]["collection"] as String;
        final id = json["id"] as String;
        return Ref(Collection(collection), id);
      }
    }
    return null;
  }

  @override
  Ref? toJson(Ref? ref) {
    if (ref?.id == null || ref?.id == "") {
      return null;
    }
    return ref;
  }
}

Example usage

@JsonSerializable()
@RefResultConverter()
@RefNulableResultConverter()
@AlertConverter()
class ShoppingListData {
  Ref accountRef;
  Ref privateUserRef;
  Ref walletRef;
  DateTime createdDate;

  Ref irregularExpensePlanRef;
  String name;
  bool campaignAlert;
  Alert? alert;

  @JsonKey(defaultValue: 0)
  int paymentMethodId = 0;
  Ref? personRef;
  Ref? corporationRef;
  @JsonKey(defaultValue: 1)
  int relevantTypeId = 1;
  Ref? paymentAccountRef;
  Ref? paymentCardRef;

  String? note;
  List<String>? photoUrlList;
  List<String>? audioUrlList;

  ShoppingListData({
    required this.accountRef,
    required this.privateUserRef,
    required this.walletRef,
    required this.createdDate,
    required this.irregularExpensePlanRef,
    required this.name,
    required this.campaignAlert,
    required this.paymentMethodId,
    required this.relevantTypeId,
    this.personRef,
    this.corporationRef,
    this.paymentAccountRef,
    this.paymentCardRef,
    this.alert,
    this.note,
    this.photoUrlList,
    this.audioUrlList,
  });

 

  factory ShoppingListData.fromJson(Map<String, dynamic> json) => _$ShoppingListDataFromJson(json);

  Map<String, dynamic> toJson() => _$ShoppingListDataToJson(this);
}

@rrousselGit
Copy link
Contributor Author

You don't need to use both snnotations at the same time anymore. remove RefNulableResultConverter and your code should work

@hasimyerlikaya
Copy link

Thank you,

Actually, I tried that way, but there is a problem.

PersonRef value should be null, but Ref.empty() value.
There should be a null check during the conversion process and if json['personRef'] is null it should be set to null, no conversion should be done.

image

@rrousselGit

@rrousselGit
Copy link
Contributor Author

It's just a thought but: is changing the dyanmicin your JsonConverter to Object?fixing the issue?

@hasimyerlikaya
Copy link

I don't think it will fix it, Ref.empty() value will be assigned because it will still be null.

There should be a null check and if the data is null the converter should not be called at all.

@dzluppin
Copy link

dzluppin commented Aug 14, 2022

Hi,

I have problem with this merge. I use two converter for a custom type wich name is Ref. I can't generate g.dart file because there is an error.

"Found more than one matching converter for Ref?."

I have this kind of issue too.

Some of my classes have both DateTime and DateTime? attributes. And because my server returned a non-standard millisecond format, I had to create a JsonConverter class for it.

Previously, approx. 6 months ago, my code generator worked fine when running build_runner using both the nullable and non-nullable versions of JsonConverter for DateTime, but now it returns an error "Found more than one matching converter for DateTime?."

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.

Allow nullable JsonConverter's to work with both nullable and non-nullable types
4 participants