dart-archive / polymer-dart

Polymer support for Dart
https://pub.dartlang.org/packages/polymer
BSD 3-Clause "New" or "Revised" License
181 stars 33 forks source link

update to work with reflectable 0.4.0 and use bestEffortReflectedType #652

Closed jakemac53 closed 8 years ago

jakemac53 commented 8 years ago

Fixes https://github.com/dart-lang/polymer-dart/issues/651, cc @eernstg

sigmundch commented 8 years ago

lgtm

eernstg commented 8 years ago

I can see that PolymerMixin and PolymerBase are non-generic, so reflectedType is safe for both pre- and post-transformed code. I don't know enough about the context to tell whether there are any dark corners in the usage of bestEffortReflectedType in the remaining locations. Do you not need to distinguish between the situation where pre-transformed code delivers a reflectedType which embeds the type arguments (because bestEffort.. uses reflectedType, which is by the way the only one that works for, say, the type of a variable in pre-transformed code), and post-transformed code delivers a dynamicReflectedType (which is the only one that works for the type of a variable in post-transformed code if its type annotation contains a type variable)? You certainly can't expect equality tests to succeed in both cases (but, e.g., something like if (myReflectedType == List || myRelfectedType == const TypeValue<List<int>>().type) .. might handle the situation for List).

jakemac53 commented 8 years ago

Yes, I have some code which can handle both List and List<int> etc. I needed that before since reflectedType was returning either of those depending on if it was pre or post transform anyways.

eernstg commented 8 years ago

OK, cool!