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

SetProperty not working when the path contains dot #333

Closed chouaibr closed 8 years ago

chouaibr commented 8 years ago

Hello, I try to update a javascript object using $.observable(myObject).setProperty('myProperty', value), but it doesnt seem to work! I think the problem comes from the dot in the key of myObject. example: var data = { "a122": 1.22, "a1.22": 1.22 }; $.observable(data).setProperty("a122", data['a122'] + 3.55); // Working $.observable(data).setProperty("a1.22", data['a1.22'] + 3.55); // Not working

See http://jsfiddle.net/HDZ5u/42/

BorisMoore commented 8 years ago

JsViews is designed to work well with JavaScript dot-separated paths for stepping through a chain of objects. "person.address.street". In a template you can write {^{:person.address.street}} to bind to the street property on the address object.

If you have person with "address.street" property, then some scenarios can be made to work - e.g. rendering in JsRender, with {{:person["address.street"]}}. But JsViews data-linking does not support that. Already the parsing of data-link expressions is very complex, and to support the above would either be impossible, or require a lot of code and a perf hit.

So it is recommended rather to design your view model to use only valid JavaScript names - and therefore be compatible with using simple dot separated paths.

See http://stackoverflow.com/a/9337047/1054484

See also https://github.com/BorisMoore/jsviews/issues/260 - which shows how setProperty can be used with dots in the names of objects - but in your scenario is asking for the leaf property to have a dot in it. I could make setProperty work in that case too, but it would be of questionable value since you could still not get data binding in the template by writing {^{:#data['a1.22']}}. Hence in your jsfiddle your had to resort to $('#string_float').html(data['a1.22']);

See also http://www.jsviews.com/#linked-paths and http://www.jsviews.com/#setprop

BorisMoore commented 8 years ago

Just for future reference, the extra line below would make setProperty work for leaf properties with dots, but would have a (small) perf cost - and would be of questionable value, given JsViews declarative data-linking does not support this scenario.

} else if (path !== $expando) {
  // Simple single property case.
  parts = object[path] ? [path] : path.split("."); // Adding this line could allow setting a leaf property with a dot in the name.
  while (object && parts.length > 1) {
      ...
chouaibr commented 8 years ago

Hi, Thanks for your quick answer. The object that i'm manipulating may potentially contain a boolean value. So i had to change slightly the patch as follows:

} else if (path !== $expando) {
  // Simple single property case.
  parts = object.hasOwnProperty(path) ? [path] : path.split("."); // Adding this line could allow setting a leaf property with a dot in the name.
  while (object && parts.length > 1) {
      ...

If you validate this code, is there any chance to include this patch in future versions of jsviews ?

BorisMoore commented 8 years ago

As I said above, I don't really want to add this. It adds only partial support for an edge case, for which there is no corresponding support in JsViews templates. And it makes relative paths like "a.b.c" become essentially ambiguous - meaning any of the following objects: currentData.a.b.c currentData.a['b.c'] currentData['a.b.'].c currentData['a.b.c']. (And the code above makes it support version 1 and 2 but not 3 or 4...)

I added the code snippet above for reference, but do not propose to include it!

chouaibr commented 8 years ago

Understood. Thank you.