BorisMoore / jsviews

Interactive data-driven views, MVVM and MVP, built on top of JsRender templates
http://www.jsviews.com/#jsviews
MIT License
855 stars 130 forks source link

observable ev.data.observeAll helper broken when path is "*" #439

Closed johan-ohrn closed 4 years ago

johan-ohrn commented 4 years ago

I've stumbled upon what I believe is a bug in the observable API when the path is set to "*".

var arr = [{a:[]}]
$.observe(arr, "[].a", function(e){console.log(e.data.observeAll, e.data.observeAll && e.data.observeAll.parents())})
$.observable(arr[0].a).insert("a")
// output
// undefined undefined

Notice how path is set to [].a observeAll helper object is missing which is consistent with the documentation.

var arr = [{a:[]}]
$.observe(arr, "[].*", function(e){console.log(e.data.observeAll, e.data.observeAll && e.data.observeAll.parents())})
$.observable(arr[0].a).insert("a")
// output
// {_path: "undefined.a", filter: undefined, path: ƒ, parents: ƒ} undefined

Notice how path is set to [].* observeAll helper object is there but its internal data seem to be corrupt. _path contains "undefined" and the .parents() method returns undefined. I'm not expecting the helper object because I'm not using a deep path, i.e. **.

var arr = [{a:[]}]
$.observe(arr, "[].**", function(e){console.log(e.data.observeAll, e.data.observeAll && e.data.observeAll.parents())})
$.observable(arr[0].a).insert("a")
// output
// {_path: "root.a", filter: undefined, path: ƒ, parents: ƒ} [{…}]

Notice how path is set to [].** observeAll helper object is there and it's working. .parent() method returns correct object.

BorisMoore commented 4 years ago

Thanks Johan. I'll be travelling in the coming period (not sure yet quite how long), so I won't be able to investigate this until after my return. I'll try to get to it then...

BorisMoore commented 4 years ago

Hi Johan. I managed to get to looking at this. Thanks for calling it out!

Here is a candidate fix:

jsviews.js.txt

It is a one-line fix on line 3321: Path = allPath ? allPath + "." + relPath : allPath;

Let me know if that works for you...

BorisMoore commented 4 years ago

Hi @johan-ohrn, did you look at this fix?

johan-ohrn commented 4 years ago

Sorry I have not. I'm struggling to remember my initial use case. I will have a look at it morrow and get back to you.

johan-ohrn commented 4 years ago

I can confirm that this fix work for my use case.

BorisMoore commented 4 years ago

OK - thanks. I'll include it in the next update...

BorisMoore commented 4 years ago

Fixed in v1.0.6