search

dart-archive / observe

Support for marking objects as observable, and getting notifications when those objects are mutated
https://pub.dartlang.org/packages/observe
BSD 3-Clause "New" or "Revised" License
13 stars 6 forks source link

Migrate from smoke to reflectable #75

Open dam0vm3nt opened 8 years ago

dam0vm3nt commented 8 years ago

I've done some work here. It is using @jsProxyReflectable from polymer-1.0 because this gives many advantages on using observe with polymer-1.0.

Also I've introduced a 'breaking change' of removing Symbol in favour of String.

eernstg commented 8 years ago

Cool -- do create some issues on package reflectable if you hit a bug!

dam0vm3nt commented 8 years ago

No bugs at the moment. Would you like me to send you a PR (for observe package I mean)?

Il giorno lun 5 ott 2015 alle ore 17:20 Erik Ernst notifications@github.com ha scritto:

Cool -- do create some issues on package reflectable if you hit a bug!

— Reply to this email directly or view it on GitHub https://github.com/dart-lang/observe/issues/75#issuecomment-145567158.

jmesserly commented 8 years ago

@dam0vm3nt Awesome! Your changes look good to me so far

eernstg commented 8 years ago

If you do have a good idea about how to fix something then a PR is of course nice, but often the big step is to be able to reproduce the problem. If we get some code (e.g., some git clone commands) such that we can do that then we're usually quite happy, and then there is no need for you to study a lot of unknown details in reflectable in order to find a fix for the bug.

Ah, there was an edit: 'for observe'. If you find problems in reflectable that can be reproduced then getting access to your code is certainly helpful.

dam0vm3nt commented 8 years ago

@jmesserly I've disabled almost all the tests and created a new one to test some basic things but if you're happy with that I'll PR my changes. They works well with another "effort" I've made to let polymer-1.0 work like the old one with respect of how observe. I mean : no need to use notifyPath or set or get or add(...) or anything because they get called for you when you change the model.

dam0vm3nt commented 8 years ago

@eernstg I was missing you until your last sentence. Yep if I hit a bug on reflectable I will certainly do it. But this issue is for observe package that it would be nice if it gets ported to reflectable instead of using the old smoke. Thanks for your kind attention anyway.

dam0vm3nt commented 8 years ago

@jmesserly submitted a PR if you're nice with having all test disabled :-/