Json_serializable.dart: 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 https://github.com/google/json_serializable.dart/pull/873

Let me try to pull it over...