canjs / can-observe

Observable objects
https://canjs.com/doc/can-observe.html
MIT License
20 stars 2 forks source link

Getters aren't properly observable #31

Closed phillipskevin closed 6 years ago

phillipskevin commented 6 years ago

Getters are not called correctly when something they depend on changes. Here is some example code:

import observe from 'can-observe';
import stache from 'can-stache';

var data = observe({
    time: new Date(),

    get hh(){
        var hr = this.time.getHours() % 12;
        return hr === 0 ? 12 : hr;
    },

    get mm(){
        return this.time.getMinutes().toString().padStart(2,"00");
    },

    get ss(){
        return this.time.getSeconds().toString().padStart(2,"00");
    }
});

setInterval(() => {
    data.time = new Date();
}, 1000);

var view = stache(`
    {{hh}}:{{mm}}:{{ss}}
`);

document.body.appendChild(
    view(data)
);

When the setInterval is hit, the getters are not called and the values are set to data.time directly:

observe

justinbmeyer commented 6 years ago

This is probably a problem with can-view-scope’s fast path

Sent from my iPhone

On Nov 2, 2017, at 2:19 PM, Kevin Phillips notifications@github.com wrote:

Getters are not called correctly when something they depend on changes. Here is some example code:

import observe from 'can-observe'; import stache from 'can-stache';

var data = observe({ time: new Date(),

get hh(){
    var hr = this.time.getHours() % 12;
    return hr === 0 ? 12 : hr;
},

get mm(){
    return this.time.getMinutes().toString().padStart(2,"00");
},

get ss(){
    return this.time.getSeconds().toString().padStart(2,"00");
}

});

setInterval(() => { data.time = new Date(); }, 1000);

var view = stache( {{hh}}:{{mm}}:{{ss}} );

document.body.appendChild( view(data) ); When the setInterval is hit, the getters are not called and the values are set to data.time directly:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

phillipskevin commented 6 years ago

Forgot to mention, if these are not getters it works correctly. If you just make them functions and call them in the template like this:

{{hh()}}:{{mm()}}:{{ss()}}
matthewp commented 6 years ago

Getter test is skipped: https://github.com/canjs/can-observe/blob/dfbdc939a1e27dcef87099009aa137545271c4a6/test.js#L309 . I would think that test should be failing.

justinbmeyer commented 6 years ago

This should be fixed, if there still is a problem, it's in can-view-scope.

getters are still not directly bindable, but they are observable (via Observation) now.

justinbmeyer commented 6 years ago

getters are directly observable now.