marcglasberg / fast_immutable_collections

Dart Package: Immutable lists, sets, maps, and multimaps, which are as fast as their native mutable counterparts. Extension methods and comparators for native Dart collections.
BSD 2-Clause "Simplified" License
213 stars 30 forks source link

Incompatible with json_serializable toJson/fromJson: issue _safeKeyFromJson #58

Open JoanSchi opened 1 year ago

JoanSchi commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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:

https://github.com/google/json_serializable.dart/issues/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 commented 2 weeks ago

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

https://github.com/JoanSchi/fast_immutable_collections/commit/7612b85865b5ef7967921e2e61ce302a77c3b522

Should I apply it? Your issue in json_serializable seems to have been fixed, right? https://github.com/google/json_serializable.dart/issues/1332

JoanSchi commented 2 weeks ago

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.