Generated Dart code expects `PbMap` instead of `Map` which is hard to create

[gryznar]

Previously it was:

class Foo extends $pb.GeneratedMessage {
  factory Foo({
    $core.Map<$core.String, Bar>? bars,
  }) {
    final $result = create();
    if (bars!= null) {
      $result.bars.addAll(bars);
    }
    return $result;
  }
  ...
}

now it is:

class Foo extends $pb.GeneratedMessage {
  factory Foo({
    $pb.PbMap<$core.String, Bar>? bars,
  }) {
    final $result = create();
    if (bars!= null) {
      $result.bars.addAll(bars);
    }
    return $result;
  }
  ...
}

Creating PbMap manually is very problematic, as it expects 2 ints (one for key and one for value). Personally I don't know how to obtain them. Situation is much simpler with PbList which just exposes .from(List<E> from) constructor

[mraleph]

I think we should probably either under the #903 change partially or - if narrower type actually leads to some benefits - we should provide a convenient way to create these.

[osa1]

Good catch -- the factory argument type does not need to be a PbMap or even a Map, it could be any iterable that yields key-value pairs with the right types.

This change was done a while ago and I don't remember the details, but it must've been unintentional. I changed how the field is represented and that effected factory argument types as well. We can and should distinguish those two.

We can make the argument type Iterable<MapEntry<K, V>>.

For now, as a work around, you can create an empty message and then add the map elements with message.mapField.addAll or message.mapField[key] = value.

[kevmoo]

We might could have flagged in the readme that the code generated here is a breaking change and could break users of the generated library.

FYI for future.

[gryznar]

We might could have flagged in the readme that the code generated here is a breaking change and could break users of the generated library.

Wasn't that just a regression? I don’t think this was supposed to be released in such a state.

[kevmoo]

@gryznar well, it's tricky. This is NOT a breaking change for THIS code. For folks using this code.

But it will generate code that could break. But folks using this library to generate code should have tests, etc to make sure things don't break on generation.

...at least I hope so.

[gryznar]

Ahh, I see. If I am not wrong, I've seen in Dart protobuf recently addes tests to verify this