json_serializable.dart icon indicating copy to clipboard operation
json_serializable.dart copied to clipboard

enum as Map keys causing key type is int

Open brian030128 opened this issue 2 years ago • 13 comments

Problem

@JsonSerializable()
class MyClass {
  final Map<Slot, String> inv;

  MyClass({required this.inv});

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

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

enum Slot {
  @JsonValue(1)
  slot1,
  @JsonValue(2)
  slot2,
  @JsonValue('3')
  slot3,
}

generated enum map

const _$SlotEnumMap = {
  Slot.slot1: 1,
  Slot.slot2: 2,
  Slot.slot3: '3',
};

Generated toJson:

instance.inv.map((k, e) => MapEntry(_$SlotEnumMap[k]!, e))

Since enums can be Map keys, and enums can be tranfered to int with JsonValue. This produces a out come that the json key is a int.

Generated fromJson:

(json['inv'] as Map<String, dynamic>).map(
        (k, e) => MapEntry($enumDecode(_$SlotEnumMap, k), e as String),
      )

I don't think this should happen, since json's key should always be a String.

A solution would be:

Generated toJson:

instance.inv.map((k, e) => MapEntry(_$SlotEnumMap[k]!.toString(), e)) //added toString()

Generated fromJson:

(json['inv'] as Map<String, dynamic>).map(
        (k, e) => MapEntry($enumDecodeJsonKey(_$SlotEnumMap, k), e as String), //uses a different decode function.
      )

enumDecodeJsonKey would be a helper specific for enum map keys, would have something like.

  for (var entry in enumValues.entries) {
    if (entry.value == source || entry.value.toString() == entry.value) {
      return entry.key;
    }
  }

If this looks nice , I'd be happy to work on this and submit a pr!

brian030128 avatar Oct 19 '23 21:10 brian030128

I think I just have a bug in the @JsonValue case. If you remove the @JsonValue(1) annotation, how does the map change?

kevmoo avatar Oct 20 '23 00:10 kevmoo

I think I just have a bug in the @JsonValue case. If you remove the @JsonValue(1) annotation, how does the map change?

The generated enum map will use their values name as keys

const _$SlotEnumMap = {
  Slot.slot1: 'slot1',
  Slot.slot2: 'slot2',
  Slot.slot3: 'slot3',
};

weeb-destroyer avatar Oct 20 '23 01:10 weeb-destroyer

@kevmoo yes, it works fine in other cases. It only happens when enums are used as json keys and is annotated with int values. It's an edgy case that might need specific handling. The solution I proposed will only transfer to the code when atleast one enum is annotated with int.

brian030128 avatar Oct 20 '23 01:10 brian030128

I think the code is correct, honestly!

The input value can be either a String '3' or an int 1 or 2 and it's converted correctly.

kevmoo avatar Oct 25 '23 01:10 kevmoo

It converts to a int, shouldn't json's keys always be strings?

brian030128 avatar Oct 25 '23 06:10 brian030128

It causes wierd errors like this.

  final object = MyClass(inv: {Slot.slot1: "ww", Slot.slot3: "ww"});
  final json = object.toJson();
  final deserialized = MyClass.fromJson(json);

throws type '_Map<Object, String>' is not a subtype of type 'Map<String, dynamic>' in type cast in the fromJson method.

Even the jsonEncode in standard lib doesn't work.

  final object = MyClass(inv: {Slot.slot1: "ww", Slot.slot3: "ww"});
  final json = object.toJson();
  final deserialized = jsonEncode(json);

throws Converting object to an encodable object failed: _Map len:2

brian030128 avatar Oct 25 '23 06:10 brian030128

Hrm...let me look again...

kevmoo avatar Oct 25 '23 17:10 kevmoo

Yeah. This is crazy complex.

Effectively, (now) you CANNOT use an enum as a Map key AND set a @JsonValue to anything other than a String.

We'd need to create some funky custom version of _$SlotEnumMap that's SPECIFIC to an enum used as a Map key where the values are pre-converted to the corresponding String values.

We CAN'T just to toString because some values are encoded with something other than their toString – like DateTime.

kevmoo avatar Oct 25 '23 23:10 kevmoo

We could/should throw an error here and say "hey, you can't use an enum here" to be more clear this is not supported.

kevmoo avatar Oct 25 '23 23:10 kevmoo

I read the doc and swa that only int and String are the supported enum values. This should make it less complex?

brian030128 avatar Oct 26 '23 01:10 brian030128

I read the doc and swa that only int and String are the supported enum values. This should make it less complex?

Not really. I could just throw in a toString but that's...cheating. And would make things more complex later.

kevmoo avatar Oct 26 '23 03:10 kevmoo

https://github.com/google/json_serializable.dart/blob/e2c8badaee4e36893e13c4073375a8ea035f2403/json_serializable/README.md?plain=1#L112-L113 The doc referenced above

I could open a pr for throwing the error, when should we throw it?

brian030128 avatar Oct 26 '23 08:10 brian030128

@SpeedReach not outdated, but it's not specific enough to handle the case where @JsonValue is used.

You could try for a PR, but it's CRAZY complex to do it cleanly. I wrote most of this package. To do the plumbing correctly will be quite tough. I appreciate the enthusiasm, but it'd be a LOT of work.

kevmoo avatar Oct 26 '23 15:10 kevmoo