Incompatible with json_serializable toJson/fromJson: issue _safeKeyFromJson

[JoanSchi]

Hi

Thank you for the great package.

Issue:
I tried to use IMap<DateTime,something> with json_serializable 6.7.0 and FIC 9.1.5. Unfortunately I get a string casting error, probably the same issue as #39. Therefore I made a example with IMap, Map with DateTime and Enum as key.
I also made a example and tested the example with a suggested solution.

Problem:
The json_serializable converts the string in the desired type, while _safeKeyToJson tries again to convert the already converted type. In my case: instead of DateTime.parse(string) it will be DateTime.parse(DateTime) at this cause the casting error. (See generated code)

Suggestion for Solution
Because the type convertion is already done by json_serializable, a possible solution could be: removing _safeKeyFromJson. This works fine and is compatible with serializable and freezed package. (Could _safeKeyFromJson be a option {bool safeKey = false/true} ?)

Temperal fix:

factory IMap.fromJson(
    Map<String, Object?> json,
    K Function(Object?) fromJsonK,
    V Function(Object?) fromJsonV,
  ) =>
      json.map<K, V>(
          // (key, value) => MapEntry(fromJsonK(_safeKeyFromJson<K>(key)), fromJsonV(value)))
          (key, value) => MapEntry(fromJsonK(key), fromJsonV(value))).lockUnsafe;

Test Example
Convertion to json and back:

void main() {
  final TestMaps testMaps = TestMaps(
      iMap: {DateTime(2023, 6, 25): 'IMap<DateTime,String>'}.lock,
      map: {DateTime(2023, 6, 25): 'Map<DateTime,String>'},
      iEnumMap: {TestEnum.testValue: 'IMap<Enum,String>'}.lock,
      enumMap: {TestEnum.testValue: 'Map<Enum,String>'});

  final Map<String, dynamic> json = testMaps.toJson();

  final fromJsonTestMap = TestMaps.fromJson(json);

  print('Output test Imap:\n ${fromJsonTestMap.toString()}');
}
```

_TestMaps:_
```
import 'package:fast_immutable_collections/fast_immutable_collections.dart';
import 'package:freezed_annotation/freezed_annotation.dart';

part 'example.g.dart';

enum TestEnum {
  testValue,
}

@JsonSerializable()
class TestMaps {
  /// The generated code assumes these values exist in JSON.
  final IMap<DateTime, String> iMap;
  final IMap<TestEnum, String> iEnumMap;
  final Map<DateTime, String> map;
  final Map<TestEnum, String> enumMap;

  TestMaps(
      {required this.iMap,
      required this.map,
      required this.iEnumMap,
      required this.enumMap});

  /// Connect the generated [_$PersonFromJson] function to the `fromJson`
  /// factory.
  factory TestMaps.fromJson(Map<String, dynamic> json) =>
      _$TestMapsFromJson(json);

  /// Connect the generated [_$PersonToJson] function to the `toJson` method.
  Map<String, dynamic> toJson() => _$TestMapsToJson(this);

  @override
  String toString() {
    return 'TestMaps(iMap: $iMap, iEnumMap: $iEnumMap, map: $map, enumMap: $enumMap)';
  }
}
```

```
// GENERATED CODE - DO NOT MODIFY BY HAND

part of 'example.dart';

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

TestMaps _$TestMapsFromJson(Map<String, dynamic> json) => TestMaps(
      iMap: IMap<DateTime, String>.fromJson(
          json['iMap'] as Map<String, dynamic>,
          (value) => DateTime.parse(value as String),
          (value) => value as String),
      map: (json['map'] as Map<String, dynamic>).map(
        (k, e) => MapEntry(DateTime.parse(k), e as String),
      ),
      iEnumMap: IMap<TestEnum, String>.fromJson(
          json['iEnumMap'] as Map<String, dynamic>,
          (value) => $enumDecode(_$TestEnumEnumMap, value),
          (value) => value as String),
      enumMap: (json['enumMap'] as Map<String, dynamic>).map(
        (k, e) => MapEntry($enumDecode(_$TestEnumEnumMap, k), e as String),
      ),
    );

Map<String, dynamic> _$TestMapsToJson(TestMaps instance) => <String, dynamic>{
      'iMap': instance.iMap.toJson(
        (value) => value.toIso8601String(),
        (value) => value,
      ),
      'iEnumMap': instance.iEnumMap.toJson(
        (value) => _$TestEnumEnumMap[value]!,
        (value) => value,
      ),
      'map': instance.map.map((k, e) => MapEntry(k.toIso8601String(), e)),
      'enumMap':
          instance.enumMap.map((k, e) => MapEntry(_$TestEnumEnumMap[k]!, e)),
    };

const _$TestEnumEnumMap = {
  TestEnum.testValue: 'testValue',
};
```

[marcglasberg]

@JoanSchi I don't actually use json_serializable. The serialization code was actually contributed by others, so I wouldn't know how to fix it or test it.

Since you already have a solution, could you please create a PR? Thanks!

[JoanSchi]

@marcglasberg

Thanks

I made PR, however I tested all the Types. BigInt DateTime, uri and String works fine, nevertheless int double bool results in a type error. Unfortenately these types are not parsed correctly by json_serializable.

I filled in a issue:

google/json_serializable.dart#1332

Depending on the response of this issue I will make a fix. (It takes a little bit longer)

Nevertheless I learned to make a PR :)

[marcglasberg]

Hi @JoanSchi could you please tell me the status of this solution?

JoanSchi@7612b85

Should I apply it? Your issue in json_serializable seems to have been fixed, right? google/json_serializable.dart#1332

[JoanSchi]

Hi @marcglasberg,

Unfortunately it is not fixed in json_serializable. For a temporal workaround I use a JsonConverter.

For now it is better to leave it as it is.

It won't last very long before macro will be introduced in Dart, with a new json_serializable based on macro's, I hope this fix the issue.

[larssn]

Need to reconsider this, now that macros have been scrapped.

[marcglasberg]

@larssn @JoanSchi Thanks so much for sharing this information.

At the moment, I'm maintaining 16 packages (https://pub.dev/publishers/glasberg.dev/packages) and actively developing https://mytext.ai/, so I haven't had the chance to fully dive into json_serializable and fix this issue. I need your help.

It would be absolutely amazing if someone familiar with json_serializable could take a closer look and help find a solution. Perhaps opening a new issue for json_serializable (since the previous one was closed) or even submitting a PR to either json_serializable or fast_immutable_collections could be a great step forward.

Huge thanks in advance, I truly appreciate any help!

[JoanSchi]

@marcglasberg @larssn
I tried if the latest json_serializable solved some problems, unfortunately not. The tuple_example.dart on json_serializable I found later use the same implementation however a tuple is quite different than a map, maybe the implementation is just not suitable for a map structure. Interestingly you can change the value1 and value2 of the example to a list: value1 will be the key list and value2 will be the value list. I will think about it, maybe this is an option.

tuple_example:
https://github.com/google/json_serializable.dart/blob/master/example/lib/tuple_example.dart

Also:
I sized down the issue to a simple class with a map for a further request on json_serializable, this class will also generate the code.

class Issue<K, V> {
  Map<K, V> map;
  Issue(
    this.map,
  );

  factory Issue.fromJson(
    Map json,
    K Function(Object?) fromJsonK,
    V Function(Object?) fromJsonV,
  ) =>
      Issue(json.map<K, V>(
          (key, value) => MapEntry(fromJsonK(key), fromJsonV(value))));

  /// Converts to JSon. Json serialization support for json_serializable with @JsonSerializable.
  Object toJson(Object? Function(K) toJsonK, Object? Function(V) toJsonV) =>
      map.map((key, value) => MapEntry(toJsonK(key), toJsonV(value)));
}

[JoanSchi]

@marcglasberg
I made a request to Support a CustomMap with the same key Type as map. For custom key the JsonConverter is an good option.

Converting to a key, value list breaks compatibility and standardization, this is not a good option.

[TimWhiting]

@JoanSchi If you are wanting to fix the issue in JsonSerializable, the PR that I made is a bit out of date, but might provide some place to start from. google/json_serializable.dart#1221. This also links to an older issue for the same problem.

It seems like the design in my PR was not well received. ("I'm REALLY worried about the complexity added here – for a pretty narrow scenario."). While I recognize that it is pretty narrow in some senses, and the solution was somewhat complex, I'm not sure how to go about making the argument that it is worth it, and I don't want to seem argumentative or forceful. Unfortunately this just encourages fragmentation in the ecosystem, since if it is too complex for the official package to maintain then someone will have to maintain a fork if they care about this. I'm happy if someone wants to refactor the PR to make it more acceptable and try again, but I no longer have the need for this in my project. Or since my PR was not well received it might be better to start from scratch and compare / contrast the approaches to see if what you come up with is simpler or not.

[JoanSchi]

@TimWhiting, Thank you, I know of your PR and it is very flexible as far I can tell :). Unfortunately if there are worries about the complexity, I hope they will consider a simple implementation with the same key type support as Map. (Serialization package is to complex for me to fix the issue)