-
Notifications
You must be signed in to change notification settings - Fork 59
question: mirrors will be disabled in 2.0 - Is "reflectable" the way to go? #98
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
Comments
I'm not laying out strategies, but it shouldn't be very hard to create a version of reflectable that uses 'build' rather than being a transformer. It would require |
Thanks for your answer. We will see what the future brings. In my opinion this question should have been answered before Dart 2.0. The organic approach is nice but the whole reflection thing is to important to be so open. Without knowing a clear strategy on reflections I can't make my packages Dart 2 ready. Rewriting the mirrors/reflection part twice is to expensive. It's really sad. |
Just a reminder: Remove support for dart:mirrors and dart:isolate from dart4web |
@MikeMitterer, how would the branch https://github.com/dart-lang/reflectable/tree/use_build work for you? You'd need to change every root library: Code generation takes place when you do There is currently no support for re-generating the code "when needed", but we're looking into that. So don't worry too much if you think "but I don't want to do |
Huuuh - Cool! Sounds great. I'll try it in the next few days. |
Thanks!I played with these samples https://github.com/MikeMitterer/dart-mdl-mustache/tree/mdlVersion/samples/browser2 Where
And... what really surprised me - everything I tried just worked! 💪 In my opinion this package should be somehow declared a the official way to handle reflections in Dart. If there is no "official-way" to handle mirrors/reflections we end up with different packages using different techniques for this - and thats something nobody wants. That's it - very clean: import 'main.reflectable.dart';
main() {
initializeReflectable();
...
} Can I share this issue in Dartisans on G+? The only thing that this lib crash was when I turned on dartdec in pubspec: Error compiling dartdevc module:reflection_test_browser2|web/web__main.js
[error] The argument type 'Iterable<ClassMirror>' can't be assigned to the parameter type 'List'. (/private/var/folders/m4/9rg16xhd2rlbln753n9zk1540000gn/T/pub_oztN7T/web/main.dart, line 69, col 42)
[error] The argument type 'Iterable<String>' can't be assigned to the parameter type 'List'. (/private/var/folders/m4/9rg16xhd2rlbln753n9zk1540000gn/T/pub_oztN7T/web/main.dart, line 71, col 36)
[error] The function expression type '(MyAnnotation) → void' isn't of type '(Object) → void'. This means its parameter or return type does not match what is expected. Consider changing parameter type(s) or the returned type(s). (/private/var/folders/m4/9rg16xhd2rlbln753n9zk1540000gn/T/pub_oztN7T/web/main.dart, line 109, col 18)
Please fix all errors before compiling (warnings are okay). This is what https://github.com/MikeMitterer/dart-mdl-mustache/blob/mdlVersion/samples/browser2/reflection/web/main.dart shows: |
Thanks for all the kind words!
You're welcome to do that! I think the issues you mention with dartdevc are standard strong mode issues (that is, the code isn't as strictly typed as it needs to be with strong mode). Compilation with dartdevc can be made to work with a few adjustments (not that they useful fixes, they just illustrate what was going wrong):
|
@eernstg I got it right that this branch will be part of the next reflectable release? |
Yes, that's the plan. I need some more feedback and maybe some adjustments, but then it should be doable. Note that we're discussing code generation strategies: For now the good choice is to always do code generation for a complete package as one step (never just one program at a time). Example: For |
OK. Generating code for the whole package sounds reasonable for me if you are talking about a lib or an app but for tests this doesn't sound very practically to me. Code generation for the above sample takes ~10secs on my machine. This is OK for the whole app but very annoying while writing unit tests. The whole idea behind UT is having quick turnaround cycles... |
I'm pushing for a nicer solution to this, but right now there's still a workaround: To generate code for a single root library (and not look at "everything else"), delete
as often as you need. When you wish to generate code for a different set of root libraries, delete |
Fwiw, the long build time of ~10s is most likely because of doing single builds (calling On the first build it has to build up an analysis context for the entire app, which can take a while, but subsequent builds will re-use the same context and update the changed files. |
Sounds like I need to know how/when to call |
Because of how this is invoked with |
Honestly, it would be a better experience long term for users to give instructions on how to write a Then they can actually get incremental builds and integrate other build steps into their pipeline as needed. |
But I have no particular need to run it with However, is it really the intention that every client package must write a |
But why does it think that the build script has been changed in a situation where the number of modified files on disk is zero?! |
No, this is not the long term goal, its just the only way to run things right now (or via pub run as reflectable is doing). The long term goal is for this script to be autogenerated based on configuration, similar to how transformers work today.
Because when running in pub run there is no way to tell what script is actually being ran, the uri you get for the root library is just a root relative http uri from the |
OK, sounds like I should just remove all references to |
If you are referring to the script in the The only really supported way today of using package:build is for each user to create their own build script inside each package that they use reflectable. |
Well, that's exactly the reason why I went for |
How would anything stop me from running
where |
It won't stop you from running that file, but you will not be able to get incremental rebuilds because it will refuse to compute the input hash of that file. We enforce the same rules on ourselves as we do on our users, because we go through the same abstractions to read/write files. |
Which file is that? |
The build script itself, |
Ah, So the point would be that client package developers need to write a |
Yep! That is the recommended approach today. |
A script like this seems to work, and could easily be copied into each client package and customized a bit:
And, of course, the client package could then in the typical setup give some fixed Branch
|
It does exist (see changelog), but it doesn't look like it made it into an official release before the 2.0.0 switch so you would have to be on a dev release. |
Update on running reflectable on our continuous testing framework: Unfortunately the idea I had Wednesday did not work. I have a better idea (it looks exactly like a better idea right now, anyway ;-), but I won't get around to try it before early January. |
Fwiw, package:build_runner is getting close to a release which would likely change how package:reflectable encourages its users to do things (although the existing way would still work). If you are waiting to publish until early January anyways it may be worth waiting for that (it should be published either before the new year or very shortly after). |
Sounds good! In any case, it wouldn't be a problem to create a new version of reflectable with a different |
The difference is that there would be no manual build.dart script at all :D. You would instead publish a |
Just sent out #107 which updates to the latest and greatest, have a look! |
Any news on this? |
I still haven't managed to make it work with continuous testing (and I'm currently at DartConf). Half of Monday is allocated for trying again. |
Hi everybody, I created a library that does what you are trying to achieve with I would be very pleased to help in anything |
Thanks @luisvt I think build_mirrors and reflectable are the two major players in this game! My problem is that reflection (mirrors) is a very central point in my packages (MDL, Dice (I'll take this from Lars... in the next few weeks.) and and in many other packages. However - it's hard to make a decision in this case... |
Did 3 half days on that continuous testing issue last week. Trying some more ideas tomorrow. |
I really don't want to bother you but do you think we'll have a chance to get reflectable until end of February? |
I have this day job doing the Dart language specification, and we're really busy putting together this Dart 2 thing, but I'll push really hard for it. |
Funny, https://pub.dartlang.org/packages/analyzer/versions/0.31.1 should exist. This is what I get with
|
Asked around about this version conflict, and the relation to the ever-present Dart 2 transition. Pub version constraints are handled in Seattle, so it'll probably be a few hours before they see it. Edit Mar 21, 9:30am: No response yet. |
The analyzer related error message means nothing. Thats a pub-solver problem. If it can't resolve a dependency it spits out this kind of message - no relation to reality. However, I have reduce my dependency to only reflectable: name: 'reflection_test_browser2'
version: 0.0.1
description: An absolute bare-bones web app.
#author: Your Name <email@example.com>
#homepage: https://www.example.com
environment:
sdk: '>=1.9.0 <2.0.0'
dependencies:
dev_dependencies:
test: any
reflectable:
#'^1.0.0'
git: git@github.com:dart-lang/reflectable.git
#path: ../_repo/reflectable Now I get Package build_runner has no versions that match >=0.7.0 <0.8.0 derived from:
- reflectable 2.0.0-dev.2.0 depends on version ^0.7.0 Again - the message itself is crap. It means just that it cant resolve SOME! dependencies. I'm running Dart 1.24.3 on this machine. I can't upgrade to 2.x because I think this would break some of my libs... I thought reflectable will also support Dart SDK < 2.x - I'm wrong? |
If there are no other dependencies, then it can only be a conflict in the Dart SDK version constraint |
@zoechi Thanks but I already spend several hours fiddling with those version problems. It really starts to piss me off. All I want is to get rid off mirrors using the reflectable package. All I do is fighting with such stupid things. |
I gave it another try:
Should work but doesn't!
So it's not "test" There are some other, Google internal, dependencies that can't be resolved on my side. |
Finally - a cloned repo with these settings works:
|
Cool, thanks! That pubspec.yaml works here, too. I'm trying to see where the important differences are, but it seems to make no difference here: With |
It's a Dart 2 thing. I switched to Dart 2.0.0-dev.40.0 - now it works. I thought there would be a nice migration path from 1.x to 2.x... However at least it works this way. Thanks. |
I hope there will be a nice migration path from 1.x to 2.x for everybody, but it's not a trivial exercise, and many issues are connected with "the context" in some sense. In any case, I hope it works better for you now, and thanks for the input! |
My package DryIce (DI) https://github.com/MikeMitterer/dryice/tree/reflectable runs now with generated mirrors - which is pretty cool but now I need "reflectable" on pub because otherwise it's not possible to |
Cool! This PR just landed 2.0.0-dev.3.0, and I can then create and publish reflectable 2.0.0 . |
Reflectable 2.0.0 is available from pub.dartlang.org now. Note that the improved support for type arguments in https://github.com/dart-lang/reflectable/tree/support_static_type_arguments_nov17 has not yet been integrated into the master branch, but it will be soon. |
!!!!! MANY THANKS! Great news. Wish you a good day. |
I mean for Dart4Web?
Please we need a clear strategy for this, essential, part of the language.
The text was updated successfully, but these errors were encountered: