Strong mode mirror support for rpc package #138
Conversation
|
If we were planning to maintain this package long term I'd probably follow this up with a refactoring of how rpc reports type problems back to clients. |
|
@jcollins-g – you've double checked the tests? I'm freshly nervous about no CI. |
|
@kevmoo Yes, and I will check a third time before submitting. :-) |
|
@jcollins-g, I tested and it works fine now in my use case. |
lib/src/config/schema.dart
Outdated
| .toString() == | ||
| 'List<MediaMessage>' && | ||
| request[prop.name] is MediaMessage) { | ||
| schema.setField(sym, [request[prop.name]]); |
There was a problem hiding this comment.
Note that here [request[prop.name]] is List<dynamic> but you want List<MediaMessage>.
Consider doing
final val = request[prop.name]; after if (request.containsKey(prop.name)).
in this case val is MediaMessage should promote val and [val] should get correct reified type.
There was a problem hiding this comment.
done, with more descriptive variable name
lib/src/config/schema.dart
Outdated
| // If in form, there is an (input[type="file"] multiple) and the user | ||
| // put only one file. It's not an error and it should be accept. | ||
| // Maybe it cans be optimized. | ||
| if (schema.type.instanceMembers[sym].returnType.reflectedType |
There was a problem hiding this comment.
I would make a global:
final Type listOfMediaMessage = reflectType(List, [MediaMessage]).reflectedType;Then here you could just do .returnType.reflectedType == listOfMediaMessages which looks better then comparing strings.
There was a problem hiding this comment.
I think we can do better, still. .returnType.reflectedType is List<MediaMessage> works and is sufficiently close to the original comparison for our needs.
There was a problem hiding this comment.
.returnType.reflectedType is List<MediaMessage> is always false because reflectedType returns Type and Type is never an instance of List<MediaMessage>.
There was a problem hiding this comment.
Ugh, right. Done.
lib/src/config/schema.dart
Outdated
| // If in form, there is an (input[type="file"] multiple) and the user | ||
| // put only one file. It's not an error and it should be accept. | ||
| // Maybe it cans be optimized. | ||
| if (schema.type.instanceMembers[sym].returnType.reflectedType |
There was a problem hiding this comment.
I think we can do better, still. .returnType.reflectedType is List<MediaMessage> works and is sufficiently close to the original comparison for our needs.
lib/src/config/schema.dart
Outdated
| .toString() == | ||
| 'List<MediaMessage>' && | ||
| request[prop.name] is MediaMessage) { | ||
| schema.setField(sym, [request[prop.name]]); |
There was a problem hiding this comment.
done, with more descriptive variable name
|
Retested with 2.0.0, 2.1.0, and 2.1.1-dev-1.0. Landing and will publish shortly. |
Fixes #133.
@supermuka
I believe this should be a complete set of changes for the rpc package to support strong-mode mirrors (dart-lang/sdk#35611). It passes all tests on Dart 2.0.0, Dart 2.1.0, and bleeding edge (with strong-mode mirrors) and seems to work just fine with dart-services.