support for _fieldPath/childPath and "observable Objects"

[bago]

Hi, I'm the ko-undomanager author and I have a feature request.. I'm willing to implement it but I'd like to have some hint from you (and to know if you think my needs are "general purpose" or not).

In the undomanager I currently keep a stack of functions that have references to the "child" and the previous value, so that they simply set the value when you "undo". Now I have to improve it so that I don't keep references to live objects because it keeps "removed" objects live and this break other things. So I'd like to change it so that I remember only the "tree-path" (as a string) and the value (again as a primitive variable/object): e.g: { path: "root.object.array[3].property", oldvalue: "5" }. This would also allow me to put the undo/redo stack on the wire (this is serializable) and do smart things.

Now my issues are: 1) _fieldName on the parents doesn't work when you use observableObjects. I have models like this one: model: { objProp: ko.observable( prop1: ko.observable(prop1Val), prop2: ko.observable(prop2Val) ), obj2Prop: ko.observable( prop1: ko.observable(prop1Val), prop2: ko.observable(prop2Val) ), } Currently the "_fieldName only applies to simple Objects or Array and in my case it finds "Functions" (observable) where it only supports Objects or Array.

2) _fieldName only applies to parents and doesn't tell me the "index" in case my child is in array... so if I loop in the parents I can build "root.object.array.property" but not "root.object.array[3].property" that I need to be able to find again the right object when I execute undo. What about implementing an option so that the watching function also receive the childPath (relative to the root of the watch( and inlcuding indexes for the arrays?

[ZiadJ]

Hi Stefano, I added a new beta version which provides the ability to add the _fieldName property to child nodes through the option tagFields(see release notes). I realized that the previous code was quite hard to read and so I refactored it a bit.

I'm not sure I got you when you said _"fieldName only applies to simple Objects..." however. As far as I know I only restricted it so that it shows up on parent nodes only. May be I missed something though. The reasoning behind was that the child being already passed as an argument one wouldn't need the field name as well to identify it. But that was before I realized it can be used to create GC friendly references.

As for the second issue you'll notice that the function watchChildren provides an argument keepOffParentList passed as true when called over arrays. I'm using it to avoid pushing both an observable and its underlying object to the parent list. In this case underlying object is discarded. Not sure if this might help but you might want to play on those values to get what you want.

I'm not sure it's a good idea to pre-tag array items with an index though. Then you would need to reassign the indexes every time an item moves or gets deleted. Otherwise you might just use ko.utils.arrayIndexOf to obtain the index during a change. By the way, just in case you hadn't noticed, the index is already sent with the item parameter when an array(not its content) is modified.

I hope this helped somehow. If you want a more in depth discussion on the matter then my Skype name is ziad.jkhan. Feel free to add me anytime.

[bago]

Thank you for the skype contact (I just added you).

About "keepOffParentList" I had a fast review of the code but I didn't get how it work.. I'll study it in the next days.

About the array index I'm currently using the "arrayIndexOf" as you suggest and I agree it is not pratical to keep updating the fieldPath on the fly: better computing it when needed.

In order to receive enough information from your library and implement my behaviour I had to change this line:

var subHasChildren = watchChildren(sub, (keepOffParentList ? null : child), parents, unwatch);

I added a "subHasChildren = true" just after this because it was not adding the _fieldName to my Object(Observable(Object)) because sometimes subHasChildren was false.

I hope I'll find some time to check your beta tomorrow! Thank you very much!

[bago]

I did a fast refactor of my code to use the reactor-beta and it seems I can make it work without patching now! Thank you!!

[ZiadJ]

Great, by the way I also rewrote the demo code for testing here: http://jsfiddle.net/vadscoo0

Thought you might just use it in your code.

Edit: For anyone interested I updated the code to add the parenthesis for observables here: http://jsfiddle.net/vadscoo0/3

[bago]

I can't use that "path computation" code directly because I also have observables in between: e.g: root.path1().path2()[4]().path3() and similar things with observables everywhere.. but I already created something similar (just checking for ko.utils.isObservable at every level).

Thank you!