dart-archive / polymer-dart

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

Investigate using observables to automatically call notifyPath #570

Closed jakemac53 closed 8 years ago

jakemac53 commented 9 years ago

Here is the really basic proof of concept I put together https://gist.github.com/jakemac53/038715f6885c29b01fc0.

This would still need to be generalized into something that automatically just works for all properties, and should probably take the form of a Behavior.

Also we need to figure out how to make this work for Lists, calling notifyPath on the whole list is to inefficient for many applications as it would create an entire new JsArray each time.

dam0vm3nt commented 9 years ago

Well, that's great. So basically there should be some transformer that will attach change notifiers to "Observable" properties.

As for list, it isn't possible to just do the same thing with ObservableList events and calling the corresponding list change API you already have (or better the notification mechanism ) ?

Il giorno ven 11 set 2015 alle ore 15:50 Jacob MacDonald < notifications@github.com> ha scritto:

Here is the really basic proof of concept I put together https://gist.github.com/jakemac53/038715f6885c29b01fc0.

This would still need to be generalized into something that automatically just works for all properties, and should probably take the form of a Behavior.

Also we need to figure out how to make this work for Lists, calling notifyPath on the whole list is to inefficient for many applications as it would create an entire new JsArray each time.

— Reply to this email directly or view it on GitHub https://github.com/dart-lang/polymer-dart/issues/570.

jakemac53 commented 9 years ago

@dam0vm3nt The issue today is that calling the list modification apis would actually end up modifying the Dart List as well as the JS Array. Since the Dart List has already been updated that would not be good :). I am sure we can come up with something here, its just going to take a little bit of time to investigate the best approach.

dam0vm3nt commented 9 years ago

yeah, I thought there was already something similar to "notifyPath" for collections I figured that was the problem. Well, keep up the good work.

Il giorno ven 11 set 2015 alle ore 16:01 Jacob MacDonald < notifications@github.com> ha scritto:

@dam0vm3nt https://github.com/dam0vm3nt The issue today is that calling the list modification apis would actually end up modifying the Dart List as well as the JS Array. Since the Dart List has already been updated that would not be good :). I am sure we can come up with something here, its just going to take a little bit of time to investigate the best approach.

— Reply to this email directly or view it on GitHub https://github.com/dart-lang/polymer-dart/issues/570#issuecomment-139555321 .

dam0vm3nt commented 9 years ago

Hello Jake maybe this is usefull. As I really need this feature I started to write this thing :

https://gist.github.com/dam0vm3nt/d50b4862ccfdacd91d11

I'm using smoke and current observable. Smoke is used for resoving symbols name.

Can you tell me if this is in the right direction ?

In your opinion there a way to replace smoke with reflectable ? I couldn't find a way to translate symbols to names.

Do you think observable will be rewritten using reflectable support ?

jakemac53 commented 9 years ago

The symbol -> string conversion is definitely an issue, probably the biggest reason this hasn't already been implemented.

One option would certainly be to make a new version of observable which uses strings instead, but I haven't had time yet to fully investigate this to see if there is a simpler path.

dam0vm3nt commented 9 years ago

Ok thanks. BTW the right gits link is

https://gist.github.com/dam0vm3nt/d50b4862ccfdacd91d11

if you want to keep an eye.

Tnx again.

dam0vm3nt commented 9 years ago

I've done some more work. Introduced "_notifySplice" for ObservableList support. Added some reflection (using smoke because it gives Symbol support too, until observable is ported to reflectable, maybe). Reflection is needed because otherwise initial values are not being notified.

It basically works but I'm stuck with two issues:

Next things : want to limit the number of "observers" (better having only one per observed instance instead of one per couple [polymer_element, observed]). Need some reworking but not too difficult.

give this thing some dignity and create a pub module

think how to avoid observing unused properties (those that are not used in templates. difficult is handling dom-repeat).

jakemac53 commented 9 years ago

using smoke because it gives Symbol support too, until observable is ported to reflectable, maybe

Keep in mind that with smoke there is no automatic transformer, to remove mirrors you would need to make a custom transformer. This probably isn't a huge deal for now, but it will probably need to be migrated to reflectable at some point.

Reflection is needed because otherwise initial values are not being notified.

In terms of Polymer, this is actually ok. You do not need to notify it of the initial values.

"created" lifecycle method is never called, so I'm using attached/detached. Can't understad why. I'm using the behaviour branch of polymer.

If you are running post-transform, there is an issue today with reflectable that is probably causing this. I have a workaround though which I will push to the behaviors branch sometime today.

binding to items properties in "dom-repeat" templates is not working. The correct number of repeats are stamped but single properties are null.

Not sure exactly what you mean here, if you can put together an example project and file an issue I can investigate.

think how to avoid observing unused properties (those that are not used in templates. difficult is handling dom-repeat).

You will need to notify about changes to any @property, regardless of if its used in the template of the element. Other elements might be binding to that property and using it in their templates/etc.

dam0vm3nt commented 9 years ago

Keep in mind that with smoke there is no automatic transformer, to remove mirrors you would need to make a custom transformer. This probably isn't a huge deal for now, but it will probably need to be migrated to reflectable at some point.

I figured but as for now this is the best option, unless observable gets patched and gives me also the string property name instead of only the symbol. I thought to do it and send a pull request.

In terms of Polymer, this is actually ok. You do not need to notify it of the initial values.

Ok, but the issue is that without reflecting observable properties I cannot attach notifiers to sub-properties (ex: "user.address.street") until they change for the first time. If a change occurs first in a subpath (ex: "street" field get changed) I don't get the notification. So I need reflection. Until (if) observable get patched I'm using smoke because it's already needed for the symbol to name convertion.

Not sure exactly what you mean here, if you can put together an example project and file an issue I can investigate.

I've created a simple test case, will post an issue on polymer-dart ...

dam0vm3nt commented 9 years ago

FYI: Published on pub and created a github repo if

https://github.com/dam0vm3nt/polymer_autonotify

dam0vm3nt commented 9 years ago

Some interesting improvements:

I'm kinda satistied with the way it is: now I'm able to use polymer-dart 1.0 just like I was used with polymer-dart 0.16. Just have to add the mixin, the transformer and everything works as expected.

jakemac53 commented 9 years ago

Nice :)

dam0vm3nt commented 9 years ago

Thanks. I would be glad and honored if this little work could be used in polymer for the next (or current) milestone if you think it is worth of it. Feel free to use it the way you want.

jakemac53 commented 9 years ago

I haven't decided if we want to include it (or anything like it) as part of the main polymer package or not, not because I disagree with the approach or anything I just think it actually makes sense as a separate package (unless there is a clear advantage to making it part of the primary package).

What I think does make sense is to advertise it as an option when we release polymer, especially since it should make the migration to 1.0 much easier for people. I can hopefully take some time to evaluate the performance impact soon as well.

jakemac53 commented 9 years ago

Also just curious, does this support nested observables or just top level properties?

dam0vm3nt commented 9 years ago

It supports nested observables and obsevables with observablelist with observable, ecc.

For example should support path like "document.items.0.title". I've just added support for breaking cycles in object graphs too.

Actually I'm facing a problem with _notifySplice. I'm calling it when an observable list get modified like this : _element.jsElement.callMethod('_notifySplice', [ jsValue(array), path, index, added, jsValue(removed)]);

but dom-repeat is not activated. Do you know of something else that should be done after updating an array ?

dam0vm3nt commented 9 years ago

Ok, it makes sense. It is nice to have such a feature pluggable or not.

Il giorno lun 21 set 2015 alle ore 17:37 Jacob MacDonald < notifications@github.com> ha scritto:

I haven't decided if we want to include it (or anything like it) as part of the main polymer package or not, not because I disagree with the approach or anything I just think it actually makes sense as a separate package (unless there is a clear advantage to making it part of the primary package).

What I think does make sense is to advertise it as an option when we release polymer, especially since it should make the migration to 1.0 much easier for people. I can hopefully take some time to evaluate the performance impact soon as well.

— Reply to this email directly or view it on GitHub https://github.com/dart-lang/polymer-dart/issues/570#issuecomment-142017902 .

jakemac53 commented 9 years ago

Fwiw, I did a really basic benchmark test and it seems like it is probably ok - at least the initial creation of elements isn't dramatically affected. In mirrors mode its pretty bad but post-transform its fine.

dam0vm3nt commented 9 years ago

It's definitively worth. Thanks. Now I'm just trying to figure out why _notifySplice is not doing it's work. Seams just like when I didn't use "JsProxy". Attaching an @Observe('collection.splices') gives me an event but the argument is null (instead of splices data)...

Il giorno lun 21 set 2015 alle ore 19:46 Jacob MacDonald < notifications@github.com> ha scritto:

Fwiw, I did a really basic benchmark test and it seems like it is probably ok - at least the initial creation of elements isn't dramatically affected. In mirrors mode its pretty bad but post-transform its fine.

— Reply to this email directly or view it on GitHub https://github.com/dart-lang/polymer-dart/issues/570#issuecomment-142056635 .

jakemac53 commented 9 years ago

@dam0vm3nt I think I know what the issue is actually, the jsValue(array) call is creating a new JsArray object, so the identity doesn't match up. As a workaround you could possibly do something like jsElement[path] instead, but really jsValue should be altered to not create a new array each time, probably just by using an expando of JsArrays that have been created already for each Dart List.

There are some other issues with this as well though, kind of hard to get into right now but essentially it would be really easy to get the Dart List and the JsArray out of sync which leads to bad things obviously. Creating a new one each time helps work around that somewhat.

dam0vm3nt commented 9 years ago

Mmh, I suspected something similar. I tried using jsValue(_element.get (path)) but nothing changed and now I understand why, I was just doing the same thing as before but more complicated :-) I'll try with jsElement[path] and see what comes out. It sounds promising.

Il giorno lun 21 set 2015 19:56 Jacob MacDonald notifications@github.com ha scritto:

@dam0vm3nt https://github.com/dam0vm3nt I think I know what the issue is actually, the jsValue(array) call is creating a new JsArray object, so the identity doesn't match up. As a workaround you could possibly do something like jsElement[path] instead, but really jsValue should be altered to not create a new array each time, probably just by using an expando of JsArrays that have been created already for each Dart List.

There are some other issues with this as well though, kind of hard to get into right now but essentially it would be really easy to get the Dart List and the JsArray out of sync which leads to bad things obviously. Creating a new one each time helps work around that somewhat.

— Reply to this email directly or view it on GitHub https://github.com/dart-lang/polymer-dart/issues/570#issuecomment-142060945 .

dam0vm3nt commented 9 years ago

I was thinking about it. If I have understood how things are working if I modify an array on dart side directly (without the list api) the JS version of it will not be automatically synced. I suspect that the same thing happens on the other direction. If things are like that I wonder if just calling the list change api instead of _notifySplice will do the trick. Essentially I'm keeping those to versions (dart and js) of the same array in sync.

I wonder what happens under the hood when two different polymer elements reference the same List. Each one will mantain its own copy of the list ? (Because jsValue will create a new array for each) ?

2015-09-21 20:25 GMT+02:00 Vittorio Ballestra vittorio.ballestra@gmail.com :

Mmh, I suspected something similar. I tried using jsValue(_element.get (path)) but nothing changed and now I understand why, I was just doing the same thing as before but more complicated :-) I'll try with jsElement[path] and see what comes out. It sounds promising.

Il giorno lun 21 set 2015 19:56 Jacob MacDonald notifications@github.com ha scritto:

@dam0vm3nt https://github.com/dam0vm3nt I think I know what the issue is actually, the jsValue(array) call is creating a new JsArray object, so the identity doesn't match up. As a workaround you could possibly do something like jsElement[path] instead, but really jsValue should be altered to not create a new array each time, probably just by using an expando of JsArrays that have been created already for each Dart List.

There are some other issues with this as well though, kind of hard to get into right now but essentially it would be really easy to get the Dart List and the JsArray out of sync which leads to bad things obviously. Creating a new one each time helps work around that somewhat.

— Reply to this email directly or view it on GitHub https://github.com/dart-lang/polymer-dart/issues/570#issuecomment-142060945 .

jakemac53 commented 9 years ago

I was thinking about it. If I have understood how things are working if I modify an array on dart side directly (without the list api) the JS version of it will not be automatically synced. I suspect that the same thing happens on the other direction. If things are like that I wonder if just calling the list change api instead of _notifySplice will do the trick. Essentially I'm keeping those to versions (dart and js) of the same array in sync.

In this case you are responding to a modification which already happened on the dart side though right?

I wonder what happens under the hood when two different polymer elements reference the same List. Each one will mantain its own copy of the list ? (Because jsValue will create a new array for each)?

If they are associated with each other via a binding, then they will share the list. If they are not and you just directly assign the list to each one, they will have separate copies (today).

dam0vm3nt commented 9 years ago

2015-09-21 22:31 GMT+02:00 Jacob MacDonald notifications@github.com:

I was thinking about it. If I have understood how things are working if I modify an array on dart side directly (without the list api) the JS version of it will not be automatically synced. I suspect that the same thing happens on the other direction. If things are like that I wonder if just calling the list change api instead of _notifySplice will do the trick. Essentially I'm keeping those to versions (dart and js) of the same array in sync.

In this case you are responding to a modification which already happened on the dart side though right?

yes, I've just tryied and it will loop. Now I'll try with your hint (jsElement[path])

I wonder what happens under the hood when two different polymer elements reference the same List. Each one will mantain its own copy of the list ? (Because jsValue will create a new array for each)?

If they are associated with each other via a binding, then they will share the list. If they are not and you just directly assign the list to each one, they will have separate copies (today).

This means that my idea will not work. When the list is shared both elements will add items to the same jsarray ...

— Reply to this email directly or view it on GitHub https://github.com/dart-lang/polymer-dart/issues/570#issuecomment-142100134 .

jakemac53 commented 9 years ago

This means that my idea will not work. When the list is shared both elements will add items to the same jsarray ...

Good point, will need to think on this one. Essentially you would need to do some bookkeeping to know if you had already applied a given change on a different element. If you have two elements which use the same list but aren't sharing the same JsArray that gets even uglier, but that would be super broken in polymer JS as well, so I guess its not really allowed?

dam0vm3nt commented 9 years ago

tried with jsElement[path] but gives me "null". I figure that's because in my test case the list is in a subpath ("model.collection") and it is not directly a subproperties of the component. This used to work in polymer 0.16 and thus I want to replicate the same behavior.

I've tryied also calling jsElement.callMethod("splice") but it loops again.

Enough random trial for now. I'll try to think of a better strategy. That Expando thing look promising.

Another thing I need to understand is why it loops when calling the "splice" (ore insertAll,ecc). Is there some observer in polymer-dart that will replicate changes back from the jsArray to the dart List ?

2015-09-21 22:44 GMT+02:00 Jacob MacDonald notifications@github.com:

This means that my idea will not work. When the list is shared both elements will add items to the same jsarray ...

Good point, will need to think on this one. Essentially you would need to do some bookkeeping to know if you had already applied a given change on a different element. If you have two elements which use the same list but aren't sharing the same JsArray that gets even uglier, but that would be super broken in polymer JS as well, so I guess its not really allowed?

— Reply to this email directly or view it on GitHub https://github.com/dart-lang/polymer-dart/issues/570#issuecomment-142103305 .

dam0vm3nt commented 9 years ago

Seams that doing that expando thing you suggested and only using _notifyPath will do the trick. Things starts to get working, but needs some more testing. I've submitted a pull request on polymer_interop, but you will probably make it better.

2015-09-21 23:03 GMT+02:00 Vittorio Ballestra vittorio.ballestra@gmail.com :

tried with jsElement[path] but gives me "null". I figure that's because in my test case the list is in a subpath ("model.collection") and it is not directly a subproperties of the component. This used to work in polymer 0.16 and thus I want to replicate the same behavior.

I've tryied also calling jsElement.callMethod("splice") but it loops again.

Enough random trial for now. I'll try to think of a better strategy. That Expando thing look promising.

Another thing I need to understand is why it loops when calling the "splice" (ore insertAll,ecc). Is there some observer in polymer-dart that will replicate changes back from the jsArray to the dart List ?

2015-09-21 22:44 GMT+02:00 Jacob MacDonald notifications@github.com:

This means that my idea will not work. When the list is shared both elements will add items to the same jsarray ...

Good point, will need to think on this one. Essentially you would need to do some bookkeeping to know if you had already applied a given change on a different element. If you have two elements which use the same list but aren't sharing the same JsArray that gets even uglier, but that would be super broken in polymer JS as well, so I guess its not really allowed?

— Reply to this email directly or view it on GitHub https://github.com/dart-lang/polymer-dart/issues/570#issuecomment-142103305 .

dam0vm3nt commented 9 years ago

"_notifyPath" was meant to be "_notifySplice"

2015-09-22 0:00 GMT+02:00 Vittorio Ballestra vittorio.ballestra@gmail.com:

Seams that doing that expando thing you suggested and only using _notifyPath will do the trick. Things starts to get working, but needs some more testing. I've submitted a pull request on polymer_interop, but you will probably make it better.

2015-09-21 23:03 GMT+02:00 Vittorio Ballestra < vittorio.ballestra@gmail.com>:

tried with jsElement[path] but gives me "null". I figure that's because in my test case the list is in a subpath ("model.collection") and it is not directly a subproperties of the component. This used to work in polymer 0.16 and thus I want to replicate the same behavior.

I've tryied also calling jsElement.callMethod("splice") but it loops again.

Enough random trial for now. I'll try to think of a better strategy. That Expando thing look promising.

Another thing I need to understand is why it loops when calling the "splice" (ore insertAll,ecc). Is there some observer in polymer-dart that will replicate changes back from the jsArray to the dart List ?

2015-09-21 22:44 GMT+02:00 Jacob MacDonald notifications@github.com:

This means that my idea will not work. When the list is shared both elements will add items to the same jsarray ...

Good point, will need to think on this one. Essentially you would need to do some bookkeeping to know if you had already applied a given change on a different element. If you have two elements which use the same list but aren't sharing the same JsArray that gets even uglier, but that would be super broken in polymer JS as well, so I guess its not really allowed?

— Reply to this email directly or view it on GitHub https://github.com/dart-lang/polymer-dart/issues/570#issuecomment-142103305 .

dam0vm3nt commented 9 years ago

I'm giving up for today. Here's what's happening now:

So we need to update the jsArray too. But if I call "splice" API instead of "_spliceNotify" then there is something (I still haven't found what) that will update again the dart list creating an infinite loop. I need to break that loop and I need to break it before changes gets applyied to the dartList twice. This means there should be a way to indicate that changes should not be reverted back to dartList (sorta like "fromAbove" for notifyPath), but it's too late now for me. Let's see tomorrow. Bye!

2015-09-22 0:01 GMT+02:00 Vittorio Ballestra vittorio.ballestra@gmail.com:

"_notifyPath" was meant to be "_notifySplice"

2015-09-22 0:00 GMT+02:00 Vittorio Ballestra <vittorio.ballestra@gmail.com

:

Seams that doing that expando thing you suggested and only using _notifyPath will do the trick. Things starts to get working, but needs some more testing. I've submitted a pull request on polymer_interop, but you will probably make it better.

2015-09-21 23:03 GMT+02:00 Vittorio Ballestra < vittorio.ballestra@gmail.com>:

tried with jsElement[path] but gives me "null". I figure that's because in my test case the list is in a subpath ("model.collection") and it is not directly a subproperties of the component. This used to work in polymer 0.16 and thus I want to replicate the same behavior.

I've tryied also calling jsElement.callMethod("splice") but it loops again.

Enough random trial for now. I'll try to think of a better strategy. That Expando thing look promising.

Another thing I need to understand is why it loops when calling the "splice" (ore insertAll,ecc). Is there some observer in polymer-dart that will replicate changes back from the jsArray to the dart List ?

2015-09-21 22:44 GMT+02:00 Jacob MacDonald notifications@github.com:

This means that my idea will not work. When the list is shared both elements will add items to the same jsarray ...

Good point, will need to think on this one. Essentially you would need to do some bookkeeping to know if you had already applied a given change on a different element. If you have two elements which use the same list but aren't sharing the same JsArray that gets even uglier, but that would be super broken in polymer JS as well, so I guess its not really allowed?

— Reply to this email directly or view it on GitHub https://github.com/dart-lang/polymer-dart/issues/570#issuecomment-142103305 .

dam0vm3nt commented 9 years ago

ok, I've found the solution. I'm using _notifySplice and change the underlying array directly without using spliceAPI from polymer. This way it's working ok. I'm committing. Hope the pull request gets approved. 'night!

dam0vm3nt commented 9 years ago

Here I'm again.

Things works very well now. I added some version tracking to avoid modifying the jsArray twice when the same list is referenced by different components either directly or indirectly by referencing an object that will reference the list.

Hope the change at polymer_interop gets approved.

2015-09-22 1:06 GMT+02:00 Vittorio Ballestra vittorio.ballestra@gmail.com:

I'm giving up for today. Here's what's happening now:

  • the dart observable list receives a new item
  • the auto notifier observer will call _notifySplice
  • finally with the expando trick the "splice" event is "fired" and dom-repeat produces its thing
  • BUT : the underlying jsArray was never updated then gets out of sync with the dart list and so dom-repeat produced content is not bound with any item

So we need to update the jsArray too. But if I call "splice" API instead of "_spliceNotify" then there is something (I still haven't found what) that will update again the dart list creating an infinite loop. I need to break that loop and I need to break it before changes gets applyied to the dartList twice. This means there should be a way to indicate that changes should not be reverted back to dartList (sorta like "fromAbove" for notifyPath), but it's too late now for me. Let's see tomorrow. Bye!

2015-09-22 0:01 GMT+02:00 Vittorio Ballestra <vittorio.ballestra@gmail.com

:

"_notifyPath" was meant to be "_notifySplice"

2015-09-22 0:00 GMT+02:00 Vittorio Ballestra < vittorio.ballestra@gmail.com>:

Seams that doing that expando thing you suggested and only using _notifyPath will do the trick. Things starts to get working, but needs some more testing. I've submitted a pull request on polymer_interop, but you will probably make it better.

2015-09-21 23:03 GMT+02:00 Vittorio Ballestra < vittorio.ballestra@gmail.com>:

tried with jsElement[path] but gives me "null". I figure that's because in my test case the list is in a subpath ("model.collection") and it is not directly a subproperties of the component. This used to work in polymer 0.16 and thus I want to replicate the same behavior.

I've tryied also calling jsElement.callMethod("splice") but it loops again.

Enough random trial for now. I'll try to think of a better strategy. That Expando thing look promising.

Another thing I need to understand is why it loops when calling the "splice" (ore insertAll,ecc). Is there some observer in polymer-dart that will replicate changes back from the jsArray to the dart List ?

2015-09-21 22:44 GMT+02:00 Jacob MacDonald notifications@github.com:

This means that my idea will not work. When the list is shared both elements will add items to the same jsarray ...

Good point, will need to think on this one. Essentially you would need to do some bookkeeping to know if you had already applied a given change on a different element. If you have two elements which use the same list but aren't sharing the same JsArray that gets even uglier, but that would be super broken in polymer JS as well, so I guess its not really allowed?

— Reply to this email directly or view it on GitHub https://github.com/dart-lang/polymer-dart/issues/570#issuecomment-142103305 .

dam0vm3nt commented 9 years ago

Huston, we got a problem.

I'm facing a new issue here: I'm testing adding items to a collections, removing and updating items fields.

It happens something strange : adding item in the middle of a collection seams to work because the page will render correctly but under the hood something gets scrambled. Infact changing the field value of one item of the collection will make another item to be render the change.

For example:

It happens that for some reason polymer will not update its mapping keys->values and so everything gets scrambled.

Using the list modification API everything works fine (event with patcher interop_polymer). What is strange is that when the item C is added at position 1 I call "splice" on the jsArray and then "_notifyPath" and this should be perfectly equivalent to calling the add element API.

Do you have any idea on how to fix this ?

2015-09-22 9:28 GMT+02:00 Vittorio Ballestra vittorio.ballestra@gmail.com:

Here I'm again.

Things works very well now. I added some version tracking to avoid modifying the jsArray twice when the same list is referenced by different components either directly or indirectly by referencing an object that will reference the list.

Hope the change at polymer_interop gets approved.

2015-09-22 1:06 GMT+02:00 Vittorio Ballestra <vittorio.ballestra@gmail.com

:

I'm giving up for today. Here's what's happening now:

  • the dart observable list receives a new item
  • the auto notifier observer will call _notifySplice
  • finally with the expando trick the "splice" event is "fired" and dom-repeat produces its thing
  • BUT : the underlying jsArray was never updated then gets out of sync with the dart list and so dom-repeat produced content is not bound with any item

So we need to update the jsArray too. But if I call "splice" API instead of "_spliceNotify" then there is something (I still haven't found what) that will update again the dart list creating an infinite loop. I need to break that loop and I need to break it before changes gets applyied to the dartList twice. This means there should be a way to indicate that changes should not be reverted back to dartList (sorta like "fromAbove" for notifyPath), but it's too late now for me. Let's see tomorrow. Bye!

2015-09-22 0:01 GMT+02:00 Vittorio Ballestra < vittorio.ballestra@gmail.com>:

"_notifyPath" was meant to be "_notifySplice"

2015-09-22 0:00 GMT+02:00 Vittorio Ballestra < vittorio.ballestra@gmail.com>:

Seams that doing that expando thing you suggested and only using _notifyPath will do the trick. Things starts to get working, but needs some more testing. I've submitted a pull request on polymer_interop, but you will probably make it better.

2015-09-21 23:03 GMT+02:00 Vittorio Ballestra < vittorio.ballestra@gmail.com>:

tried with jsElement[path] but gives me "null". I figure that's because in my test case the list is in a subpath ("model.collection") and it is not directly a subproperties of the component. This used to work in polymer 0.16 and thus I want to replicate the same behavior.

I've tryied also calling jsElement.callMethod("splice") but it loops again.

Enough random trial for now. I'll try to think of a better strategy. That Expando thing look promising.

Another thing I need to understand is why it loops when calling the "splice" (ore insertAll,ecc). Is there some observer in polymer-dart that will replicate changes back from the jsArray to the dart List ?

2015-09-21 22:44 GMT+02:00 Jacob MacDonald notifications@github.com:

This means that my idea will not work. When the list is shared both elements will add items to the same jsarray ...

Good point, will need to think on this one. Essentially you would need to do some bookkeeping to know if you had already applied a given change on a different element. If you have two elements which use the same list but aren't sharing the same JsArray that gets even uglier, but that would be super broken in polymer JS as well, so I guess its not really allowed?

— Reply to this email directly or view it on GitHub https://github.com/dart-lang/polymer-dart/issues/570#issuecomment-142103305 .

dam0vm3nt commented 9 years ago

I've also tried calling _originalSplice (by making it available from polymer_array_methods.html and calling that instead of updating the jsArray and calling _notifyPath.This should be identical of using the "array API" but there is something that I'm still missing because nothing has changed.

jakemac53 commented 9 years ago

Unrelated to the current issues you are having but another thing I was just thinking about - you should try this in the no-proxy branch of polymer. It uses a pretty different strategy for things, basically it just hooks into the _propertyChanged method and applies changes on the dart side using that instead of using a proxy object for the element to apply changes. This probably has some consequences for what you are doing (its more likely to re-apply changes).

We should be able to get around this, but just an fyi.

dam0vm3nt commented 9 years ago

Thank you for the advice. I will try and see what happens. I'm almost sure that you're right and changes will get reapplied givin me an infinite loop. Do you already know if this Is it going to be the final strategy ? In that case I will switch on that branch immediatly

jakemac53 commented 9 years ago

@dam0vm3nt yes it is almost certainly going to be the final strategy - it saves a fairly significant amount of time in the creation of each element. The only reason it wouldn't the final strategy is if we found some reason that it wasn't feasible (but so far everything looks ok - all tests are passing).

jakemac53 commented 9 years ago

Also, I sent out https://github.com/dam0vm3nt/polymer_autonotify/pull/1. That resolves a few errors I was seeing in checked mode. In terms of your other issue I noticed that weird behavior as well. The really weird thing is if you try to modify the item that just got added, it actually modifies the one that used to be at that index. All other items can be modified like normal. Not sure if this is a polymer bug or what yet.

jakemac53 commented 9 years ago

One hint for the no-proxy branch, instead of calling _notifySplice you could call notifyPath('property.splices', ...). The 2nd arg would be a polymer js splices object, which if you add the _applied = true property to it will be ignored by polymer dart.

dam0vm3nt commented 9 years ago

Tnx! Just merged.

2015-09-22 16:54 GMT+02:00 Jacob MacDonald notifications@github.com:

Also, I sent out dam0vm3nt/polymer_autonotify#1 https://github.com/dam0vm3nt/polymer_autonotify/pull/1. That resolves a few errors I was seeing in checked mode. In terms of your other issue I noticed that weird behavior as well. The really weird thing is if you try to modify the item that just got added, it actually modifies the one that used to be at that index. All other items can be modified like normal. Not sure if this is a polymer bug or what yet.

— Reply to this email directly or view it on GitHub https://github.com/dart-lang/polymer-dart/issues/570#issuecomment-142312725 .

dam0vm3nt commented 9 years ago

Ok I'll switch it.

2015-09-22 17:04 GMT+02:00 Jacob MacDonald notifications@github.com:

One hint for the no-proxy branch, instead of calling _notifySplice you could call notifyPath('property.splices', ...). The 2nd arg would be a polymer js splices object, which if you add the _applied = true property to it will be ignored by polymer dart.

— Reply to this email directly or view it on GitHub https://github.com/dart-lang/polymer-dart/issues/570#issuecomment-142316197 .

dam0vm3nt commented 9 years ago

@jakemac53 is the "no-proxy" branch already including the fix for the issue causing the @behaviors no to be applied correctly ? After switch seams that the mixin is not applied anymore but calling PolymerAutoNotifySupportMixin.created(this); in the created constructor esplicitly it works again...

dam0vm3nt commented 9 years ago

.... and I get the infinite loop :-(

jakemac53 commented 9 years ago

It might not have been rebased recently on the behaviors branch, I am working on cleaning up and merging a bunch of other stuff today if you want to take a breather for a bit, I should be able to get it all in sync soon. I think everything will most likely end up being merged into a new branch, 1.0.0-rc1 or something along those lines.

dam0vm3nt commented 9 years ago

One hint for the no-proxy branch, instead of calling _notifySplice you could call notifyPath('property.splices', ...). The 2nd arg would be a polymer js splices object, which if you add the _applied = true property to it will be ignored by polymer dart.

Is it going to work just notiying the splices ? I though I had to update the jsArray too ... I'll try it I'm curious.

Should I use also another polymer_interop branch ?

dam0vm3nt commented 9 years ago

One hint for the no-proxy branch, instead of calling _notifySplice you could call notifyPath('property.splices', ...). The 2nd arg would be a polymer js splices object, which if you add the _applied = true property to it will be ignored by polymer dart.

Well, I can just work with explicit calls for now. Just wanted to be sure it was only that and not another issue. Tnx.

jakemac53 commented 9 years ago

Is it going to work just notiying the splices ? I though I had to update the jsArray too ... I'll try it I'm curious.

I am not actually sure on this one, but I have a feeling you will need to update the JsArray manually like before.

Should I use also another polymer_interop branch ?

You probably want to point at your github repo for now until that PR gets merged.

dam0vm3nt commented 9 years ago

good news is that It is working on no-proxy branch too. I had yo put an ugly "global variable horrible hack" to disable propertyChangeHook altogether during notification because a didn't find a better way to do it. But it works.

...

bool polymerDartSyncDisabled=false;

void _setUpPropertyChanged() {
  _polymerDart['propertyChanged'] = (instance, String path, newValue) {
    if (polymerDartSyncDisabled) {
      return;
    }

 ...

The flag is set and reset just befor/after calling notifyPath or splice. I could not find a way of passing an extra argument without changing to many things. Maybe I can use another expando on the instance argument but I'm not coinvinced.

Bad news is that there is still the weird issue about item indexes that's driving me mad :-(

dam0vm3nt commented 9 years ago

I'm badly stuck. sigh. Having brutally shut down the js -> dart syncronization during notify path I ended calling PolymerElement.set and PolymerElement.splice that should be exaclty has if I called them in the beginning but weird things continue to happen. I'll leave things in stand-by until I come with some good idea on how to unstuck.

This is what is happening :

https://youtu.be/HpRWKj6ybFo

It should always update the element at position 1 but after inserting elements before that the consecutive update goes crazy

dam0vm3nt commented 9 years ago

I think I've hitted a polymer-dart bug :

https://github.com/dart-lang/polymer-dart/issues/591

Possibly this is the source of weirdness.

dam0vm3nt commented 9 years ago

@jacob314 Extra! extra! read all about it!

Status update: