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

[shilangyu]

• Dart version: Dart SDK version: 2.12.0 (stable) (Thu Feb 25 19:50:53 2021 +0100) on "linux_x64"
• json_serializable version: 4.0.2

When creating a custom JsonConverter that handles nullable types, it will not be applied to non-nullables. Example, consider this:

import 'package:json_annotation/json_annotation.dart';

part 'mydata.g.dart';

class ForceUtcDateTime implements JsonConverter<DateTime?, String?> {
  const ForceUtcDateTime();

  @override
  DateTime? fromJson(String? json) => json == null
      ? null
      : DateTime.parse('$json${json.endsWith('Z') ? '' : 'Z'}');

  @override
  String? toJson(DateTime? json) => json?.toIso8601String();
}

@JsonSerializable()
@ForceUtcDateTime()
class MyData {
  final DateTime created;
  final DateTime? updated;

  const MyData({required this.created, this.updated});

  factory MyData.fromJson(Map<String, dynamic> json) => _$MyDataFromJson(json);
  Map<String, dynamic> toJson() => _$MyDataToJson(this);
}

A custom DateTime converter is created that handles DateTime?. The generated code is as follows:

MyData _$MyDataFromJson(Map<String, dynamic> json) {
  return MyData(
    created: DateTime.parse(json['created'] as String),
    updated: const ForceUtcDateTime().fromJson(json['updated'] as String?),
  );
}

Map<String, dynamic> _$MyDataToJson(MyData instance) => <String, dynamic>{
      'created': instance.created.toIso8601String(),
      'updated': const ForceUtcDateTime().toJson(instance.updated),
    };

I would expect both created and updated to be handled by my ForceUtcDateTime converted, not just updated

If this is intended, I would like to know what is the expected way of handling such cases.

[Ludonope]

Having same issue, also impacts when the type is used in a list or anything

// my_data.dart
@JsonSerializable()
@MyTypeConverter()
class MyData {
  late MyType nonNullable;
  MyType? yesNullable;
  List<MyType>? someList;

  const MyData();

  factory MyData.fromJson(Map<String, dynamic> json) => _$MyDataFromJson(json);
  Map<String, dynamic> toJson() => _$MyDataToJson(this);
}

// my_data.g.dart
MyData _$MyDataFromJson(Map<String, dynamic> json) {
  return MyData()
    ..nonNullable = const MyTypeConverter().fromJson(json['nonNullable'] as String)
    ..yesNullable = const MyTypeConverter().fromJson(json['nonNullable'] as String)
    ..someList = (json['someList'] as List<dynamic>?)
        ?.map((e) => const MyTypeConverter().fromJson(e as String))
        .toList();
}

Map<String, dynamic> _$MyDataToJson(MyData instance) => <String, dynamic>{
      'nonNullable': const MyTypeConverter().toJson(instance.nonNullable),
      'yesNullable': const MyTypeConverter().toJson(instance.yesNullable),
      'someList': instance.someList
          ?.map(const MyTypeConverter().toJson)
          .toList(),
};

I end up with two errors:

The argument type 'MyType?' can't be assigned to the parameter type 'MyType'. (my_data.g.dart:4)
A value of type 'List<MyType?>?' can't be assigned to a variable of type 'List<MyType>?'. (my_data.g.dart:6)

[kevmoo]

The short fix is to create another converter that handles the nullable case.

we could set it up so that we handle nullable values with non-nullable converters like we do for DateTime, etc. And just pass them through.

I'll need to think about that case.

[shilangyu]

Here is why I think this use-case should be supported: consider a modified example from my initial comment:

import 'package:json_annotation/json_annotation.dart';

part 'asd.g.dart';

class MyType {
  final int val;

  const MyType(this.val);

  factory MyType.fromJson(String json) => MyType(int.parse(json));

  String toJson() => val.toString();
}

@JsonSerializable()
class MyData {
  final MyType created;
  final MyType? updated;

  const MyData({required this.created, this.updated});

  factory MyData.fromJson(Map<String, dynamic> json) => _$MyDataFromJson(json);
  Map<String, dynamic> toJson() => _$MyDataToJson(this);
}

The only real difference is using a custom type with fromJson and toJson methods. This example correctly serializes and handles both MyType and MyType?. So if this works, I think the converter example should also work.

On a similar note, perhaps there is a better way of creating custom serializations for existing types? Because that is what I am using a Converter for, but it's tedious to decorate all of my classes with this converter.

[limenote135]

With defaultValue, generated code is redundant.

@JsonSerializable()
@SampleConverter()
class Sample {
  @JsonKey(defaultValue: 1)
  final int val;

  const Sample({this.val = 1});

  factory Sample.fromJson(Map<String, dynamic> json) => _$SampleFromJson(json);
}

class SampleConverter implements JsonConverter<int, String?> {
  const SampleConverter();

  @override
  int fromJson(String? json) => json == null ? 1 : int.parse(json);

  @override
  String? toJson(int object) => "$object";
}

Sample _$SampleFromJson(Map<String, dynamic> json) {
  return Sample(
    val: const SampleConverter().fromJson(json['val'] as String?) ?? 1, // warning: The left operand can't be null
  );
}

So, I want to use not JsonConverter<int, String?> but JsonConverter<int?, String?>.

[comigor]

Same for JsonKey's fromJson/toJson.

The following code doesn't work:

// GENERATED CODE - DO NOT MODIFY BY HAND
// @dart = 2.12

import 'package:json_annotation/json_annotation.dart';
part 'myclass.g.dart';

String _fromJson(String _) => '';
String _toJson(String _) => '';

@JsonSerializable(explicitToJson: true)
class MyClass extends JsonSerializable {
  MyClass();

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

  @JsonKey(fromJson: _fromJson, toJson: _toJson)
  late String str;

  @JsonKey(fromJson: _fromJson, toJson: _toJson)
  String? strNullable;

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

But, thinking about it, maybe it should work, and yield something similar to the following? (considering nullability before running the fn)

// GENERATED CODE - DO NOT MODIFY BY HAND
// @dart=2.12

part of 'myclass.dart';

// **************************************************************************
// JsonSerializableGenerator
// **************************************************************************

MyClass _$MyClassFromJson(Map<String, dynamic> json) {
  return MyClass()
    ..str = _fromJson(json['str'] as String)
    ..strNullable = json['strNullable'] == null
        ? null
        : _fromJson(json['strNullable'] as String);
}

Map<String, dynamic> _$MyClassToJson(MyClass instance) => <String, dynamic>{
      'str': _toJson(instance.str),
      'strNullable': instance.strNullable == null
          ? null
          : _toJson(instance.strNullable as String),
    };

[kevmoo]

I just did some work here in #873

Let me try to pull it over...

[kevmoo]

This is doable, but easy to work-around. Not considering it a high priority fix.

[rrousselGit]

If we're going with #1072 (comment) then I think this issue will be a necessary step

Otherwise this means that third-party packages defining reusable JsonConverters would have to define the same converter twice for a nullable variant:

@JsonSerizable(converters: [DocumentReferenceConverter(), NullableDocumentReferenceConverter()])

Also I think this behavior causes quite a bit of pain as it's not clear to users what the problem is. I've received a few issues on Freezed where folks had trouble with this.

Assuming #1072 (comment) is the way forward, I'd be more than happy to work on this issue while working on #1072 (comment).

---

What I have in mind is, if folks define a non-nullable JsonConverter AND if the JSON key is non-nullable too:

class Foo {}

class FooConverter extends JsonConverter<Foo, String> // OK

class NullableJsonKey extends JsonConverter<Foo, String?> // KO

Then json_serializable would take care of handling null implicitly such that for:

@JsonSerializable()
class Example {
  Example({required this.nonNullable, required this.nullable});

  @FooConverter()
  Foo nonNullable;

  @FooConverter()
  Foo? nullable;
}

then ends up doing:

Example _$ExampleFromJson(Map<String, dynamic> json) => Example(
      nonNullable: const FooConverter().fromJson(json['foo'] as String),
      // TODO maybe hoist `json['foo']` in a variable
     nullable: json['foo'] == null ? null : const FooConverter().fromJson(json['foo'] as String),
    );

Map<String, dynamic> _$ExampleToJson(Example instance) => <String, dynamic>{
      'nonNullable': const FooConverter().toJson(instance.foo),
      // TODO maybe hoist `instance.foo` in a variable
      'nullable': instance.foo == null ? null : const FooConverter().toJson(instance.foo!),
    };

[Afsal102]

Folks was this fixed 🤔 , i am still able to reproduce this with the same example but i was using custom annotations at field level

unsure if i am missing something though 😅

using the example from @shilangyu,

class ForceUtcDateTime implements JsonConverter<DateTime?, String?> {
  const ForceUtcDateTime();

  @override
  DateTime? fromJson(String? json) => json == null
      ? null
      : DateTime.parse('$json${json.endsWith('Z') ? '' : 'Z'}');

  @override
  String? toJson(DateTime? json) => json?.toIso8601String();
}

@JsonSerializable()
class MyData {
  @ForceUtcDateTime()
  final DateTime createdAt;

  const MyData({required this.createdAt,});

  factory MyData.fromJson(Map<String, dynamic> json) => _$MyDataFromJson(json);
  Map<String, dynamic> toJson() => _$MyDataToJson(this);
}

Versions:

json_serializable: 6.3.0
json_annotation: 4.6.0

[wwwdata]

@Afsal102 you have to use the non nullable types in your converter. It will then also work for nullable fields.

[Afsal102]

@wwwdata yea i am ware of that, i was trying to avoid having two separate converters on for nullable and other for non-nullable, unsure if the mention PR fixes the one like i have mentioned though 🤔

[wwwdata]

@Afsal102 the non nullable one will also work for nullable fields in the latest version.

[Afsal102]

i see left me give it a try thanks for the help

[shilangyu]

I now see my issue description is completely backwards. Of course I meant "Allow non-nullable JsonConverter's to work with both nullable and non-nullable types". Luckily everyone seemed to have gotten what I actually meant and the correct version was implemented. Sorry for the confusion.

[Afsal102]

@wwwdata thanks for the help it worked haa forgot to update 🙇

[ljmatan]

For anyone not being able to differentiate between nullable and non-nullable types, the workaround is as below:

@JsonSerializable(
  converters: [
    ExampleSerializationHandlerString(),
    ExampleSerializationHandlerBool(),
    ExampleSerializationHandlerInt(),
    ExampleSerializationHandlerDouble(),
    ExampleSerializationHandlerMap(),
  ],
)
@ExampleSerializationHandlerStringNullable()
@ExampleSerializationHandlerBoolNullable()
@ExampleSerializationHandlerIntNullable()
@ExampleSerializationHandlerDoubleNullable()
@ExampleSerializationHandlerMapNullable()

This is with the latest json_serializable (6.8.0) and json_annotation (4.9.0) versions at the time of this writing.