Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enum as Map keys causing key type is int #1362

Open
SpeedReach opened this issue Oct 19, 2023 · 13 comments
Open

enum as Map keys causing key type is int #1362

SpeedReach opened this issue Oct 19, 2023 · 13 comments

Comments

@SpeedReach
Copy link

SpeedReach commented Oct 19, 2023

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!

@kevmoo
Copy link
Collaborator

kevmoo commented Oct 20, 2023

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

@weeb-destroyer
Copy link

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',
};

@SpeedReach
Copy link
Author

SpeedReach commented Oct 20, 2023

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

@kevmoo
Copy link
Collaborator

kevmoo commented Oct 25, 2023

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.

@SpeedReach
Copy link
Author

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

@SpeedReach
Copy link
Author

SpeedReach commented Oct 25, 2023

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

@kevmoo
Copy link
Collaborator

kevmoo commented Oct 25, 2023

Hrm...let me look again...

@kevmoo
Copy link
Collaborator

kevmoo commented Oct 25, 2023

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
Copy link
Collaborator

kevmoo commented Oct 25, 2023

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.

@SpeedReach
Copy link
Author

SpeedReach commented Oct 26, 2023

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

@kevmoo
Copy link
Collaborator

kevmoo commented Oct 26, 2023

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.

@SpeedReach
Copy link
Author

Annotate `enum` values with [`JsonValue`] to specify the encoded value to map
to target `enum` entries. Values can be of type [`String`] or [`int`].

The doc referenced above

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

@kevmoo
Copy link
Collaborator

kevmoo commented Oct 26, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants