grpc / grpc-dart

The Dart language implementation of gRPC.
https://pub.dev/packages/grpc
Apache License 2.0
861 stars 271 forks source link

Use `Map.of` instead of `Map.from` in grpc client. #724

Closed lrhn closed 2 months ago

lrhn commented 4 months ago

Map.of creates a new map with the same keys, values and type as the original map, when used without type arguments or context type, where Map.from creates a Map<dynamic, dynamic>. (This code failed on an attempt to make Map.unmodifiable be more strictly typed, like Map.of instead of Map.from, showing that an intermediate map had type Map<dynamic, dynamic> unnecessarily).

Same for using List.of instead of List.from.

The new code should be (microscopically) more efficient and type safe, and is forwards-compatible with a stronger type on Map.unmodifiable.

(The code can be optimized more. For example List.of(list1)..addAll(list2) can be just list1 + list2 or [...list1, ...list2], both of which may know the total number of elements when doing the initial list allocation. This is a minimal change to allow the type changes for .unmodifiable to get past this very initial blocker in internal tests.)

linux-foundation-easycla[bot] commented 4 months ago

CLA Signed

The committers listed above are authorized under a signed CLA.

github-actions[bot] commented 4 months ago

PR Health

Breaking changes :heavy_check_mark: | Package | Change | Current Version | New Version | Needed Version | Looking good? | | :--- | :--- | ---: | ---: | ---: | ---: | |grpc|None|4.0.1|4.0.2|4.0.1|:heavy_check_mark:|
Changelog Entry :heavy_check_mark: | Package | Changed Files | | :--- | :--- | Changes to files need to be [accounted for](https://github.com/dart-lang/ecosystem/wiki/Changelog) in their respective changelogs.
Coverage :heavy_check_mark: | File | Coverage | | :--- | :--- | |lib/src/client/call.dart| :green_heart: 85 % | |lib/src/shared/status.dart| :green_heart: 59 % | This check for [test coverage](https://github.com/dart-lang/ecosystem/wiki/Test-Coverage) is informational (issues shown here will not fail the PR).
API leaks :warning: The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API. | Package | Leaked API symbols | | :--- | :--- | |grpc|Any
$1.Duration
ServerHandler| This check can be disabled by tagging the PR with `skip-leaking-check`.
Package publish validation :heavy_check_mark: | Package | Version | Status | | :--- | ---: | :--- | | package:grpc | 4.0.2 | **ready to publish** | Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.
mosuem commented 2 months ago

@lrhn Can these changes be merged?

lrhn commented 2 months ago

Well, now it has conflicts! But otherwise yes. Fixing conflicts. ... and done. Someone had made a 4.0.1, so this is now 4.0.2.

Feel free to commit. (I don't have commit rights to the repo.)