BorisMoore / jsviews

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

observe "array^*" (not "array^**") #408

Closed ainglese closed 5 years ago

ainglese commented 6 years ago

when using:

var model = {
    list: [
        {  qty:0, attributes:[...] },
        {  qty:1, attributes:[...] }
    ]
}
$.observe(model, 'list^**', handler)

handler gets correctly invoked when everything in list changes.

the problem is, that binding performance get worse when attributes property has many level in depth. because i just need to listen to "qty" property, i've tried:

$.observe(model, 'list^*', handler1)
$.observe(model, 'list^qty', handler2)

but none of the two works;

here's a fiddle: https://jsfiddle.net/wcro58mL/1/

am i missing something? thanks

BorisMoore commented 6 years ago

There is no path syntax for specific properties of any array item. (That would require that each time a new item is inserted or removed from the array, internal smarts would need to add/remove listeners for "qty" on the item.)

So either you need to use "**" or observeAll and then test for eventArgs.path==="qty" in the handler (and otherwise simply return), or you would need to write code yourself:

ainglese commented 6 years ago

thank you for your fast reply!

i think i've to go with the "code yourself" way, because the problem origin from a performance problem with observeAll.

ainglese commented 6 years ago

can you confirm there's no syntax also for "property exclusion", something like observeAll but attribute property?

eugene-panferov commented 6 years ago

just another reminder in the topic. when you hit the boundaries of the framework, you should seriously consider the logical elegance or even the correctness of your data model.

ainglese commented 6 years ago

@eugene-panferov of course, it's the first thing we did. But our model isn't that complex, it's a document, an object with a rows property. in each row there are many property but one of this is "custom_data", a prop where we can store arbitrary json that is used to store customized information related to the document. We need to observe change to trigger autosave and do stuff like validation.

We can't predit the structure in the custom_data field, but sure we don't need to observe that piece of information. We faced the problem when a customer started to store big structure in that field in combination with hundreds of rows.

we already know we can:

the last one seems to us the fastest way and the less impacting to the user.

if you have a suggestion on how to reenginer the model, your feedback would be really appreciated!

ainglese commented 6 years ago

i've come up with something like this (sure there are better names):

https://jsfiddle.net/wcro58mL/20/

observeArrayProps('ns', model.list, 'qty', function(){ .. });
unobserveArrayProps('ns', model.list);

    function observeArrayProps() {

        var observeArgs = Array.prototype.slice.call(arguments);
        var ns = arguments[0];
        var array = arguments[1];

        function arrayObserver(e, o) {
            if (o.change === 'insert') {
                for (var i = 0; i < o.items.length; i++) {
                    observeArgs[1] = o.items[i];
                    $.observe.apply(null, observeArgs);
                }
            }
            else if (o.change === 'remove') {
                for (var i = 0; i < o.items.length; i++) {
                    observeArgs[1] = o.items[i];
                    $.unobserve.apply(null, observeArgs);
                }
            }
        }

        $.observe(array, arrayObserver);

        for (var i = 0; i < array.length; i++) {
            observeArgs[1] = array[i];
            $.observe.apply(null, observeArgs);
        }

    }

    function unobserveArrayProps() {

        var observeArgs = Array.prototype.slice.call(arguments);
        var ns = arguments[0];
        var array = arguments[1];

        $.unobserve(ns, array);
        for (var i = 0; i < array.length; i++) {
            observeArgs[0] = array[i];
            $.unobserve.apply(null, observeArgs);
        }

    }
BorisMoore commented 6 years ago

That approach looks correct - so you should be able to do it that way - as long as you don't use $.observable().refresh() - or else you can add support for that too.

But there is a filtering feature built in which is undocumented right now. (So it could change in the future). It looks like this:

$.observable(model.list).observeAll(
function handler() {
  do stuff
},
function filter(allPath, object, parentObs) {
  return !/]\./.test(allPath); // Don't observe any object with `].` in the allPath string.
});

The filter function is called for each object in the hierarchy and you return false if you don't want to observe that object (or any of its property objects). In your example it will be called for each of the attributes objects, with allPath "root[].attributes". Return false to tell it not to recursively do an observeAll on that object.

BorisMoore commented 6 years ago

Actually your code above might be fine with .refresh() too, since .refresh() actually triggers a corresponding sequence of insert remove or move operations.

I am looking at a possible string path based approach along the lines of

$.observe(model, 'list.[].qty', handler);

Not sure yet if it can be achieved appropriately - we'll see. I'll let you know...

ainglese commented 6 years ago

thank you very much @BorisMoore! i've updated the fiddle to also test the refresh; after the refresh, the handler still gets called. https://jsfiddle.net/wcro58mL/22/

BorisMoore commented 6 years ago

@ainglese: Here is a candidate update: jsviews.js.txt.

It supports the following syntax:

Observe changes of item.qty for all current items in list:

$.observe('ns', model.list, '[].qty', handler);

Observe changes of item.qty for all items in list - including new items if list changes - and remove handlers for removed items:

$.observe('ns', model.list, '[]^qty', handler);

I went ahead with this not only for your scenario, but also for other scenarios I wanted to support, such as having a computed obersvable foo(...) and wanting to set depends paths such as foo.depends = "some.list.[]^*" (for example "#parent.data.[]^*")

Let me know if it works well for you.

ainglese commented 6 years ago

thats great! thanks i'll let you know as soon as i try it

BorisMoore commented 5 years ago

This has been fixed in release v0.9.91.