-
Notifications
You must be signed in to change notification settings - Fork 49
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
rpc package tests broken with "not a subtype" errors in 2.2+ #133
Comments
Uff.. I was thinking that it was doing something wrong! Using rpc 0.6.0 with sdk 2.1.0-dev.8.0 , in my code is thrown the following error:
With sdk 2.0.0 it works. |
@supermuka I suggest reverting to 2.1.0-dev.7.1 (that seems to still have the good behavior) if you need the 2.1 branch. Hopefully the referenced SDK bug is getting enough attention, I'll keep advocating for it. At the very least, if the rpc package itself is doing something wrong we need to know what it is so we can change it. |
Investigation in dart-lang/sdk#35009 revealed a way out for the rpc package: a refactor changing how the rpc package instantiates objects for clients to take into account their types with the mirror system. (A sample of some of the work required is in https://github.com/dart-lang/rpc/compare/experimental-mirrors-refactor). |
Just a thought, without depth: maybe it could be an alternative to make a refactory of the rpc package by changing the method for serialization, from mirror to code generated as a json_serialization (https://pub.dartlang.org/packages/json_serializable) does. I think we could have a performance improvement in runtime too. |
@supermuka It's certainly possible, though at that point I might look to a transition to grpc for my project (dart-services, the DartPad backend) rather than redesigning this package. grpc already uses generated code -- watching the rest of the dart ecosystem that seems to be the way folks are heading. |
@supermuka Just to let you know, we've decided to ship a version of rpc with a more complete fix for this, since the mirrors change is going to go into a future version of 2.1 stable. I'll try and get a PR set up for this in the next day or so and if you'd like I'd appreciate your feedback on it as a fellow customer of the package. |
@jcollins-g, I tested and it works fine now, including generic problem #134, using the branch |
See dart-lang/sdk#35009.
The text was updated successfully, but these errors were encountered: