Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Order observation updates by their depth automatically #125

Closed
justinbmeyer opened this issue Jan 3, 2018 · 5 comments · Fixed by #155
Closed

Order observation updates by their depth automatically #125

justinbmeyer opened this issue Jan 3, 2018 · 5 comments · Fixed by #155

Comments

@justinbmeyer
Copy link
Contributor

justinbmeyer commented Jan 3, 2018

tldr; Observations created inside other observations should always be entered in the queue after the outer observation. If the outer observation is updated, the child observation should be "stopped" (all handlers unbound and stops listening on its dependencies).

This might eliminate one of the core abilities of nodeLists - the need to recursively unbinding child nodelists!

Background

Take some example stache code like:

{{#if show}}
    <div>{{foo}}</div>
{{/if}}

This gets compiled into the following pseudo observations:

First, an ifBlock with a nested LOGIC block. The LOGIC observation is created
so ifBlock is not rerun over and over if show were to change from 1 to 2 to 3, etc.

var ifBlock = new Observation( () => {
    var LOGIC = new Observation(() => { return !!vm.show })
    
    if(LOGIC.get()) {
        return  options.fn()
    } else {
        return options.inverse();
    }
})

Second, options.fn (and options.inverse) are compiled to something like:

option.fn = function(){
    
    foo = new Observation(() => vm.foo , {canRecordObservation: false});
    return frag("<div/>", live.text(textNode, foo))
}

Note the canRecordObservation on the foo observation. This is used to prevent ifBlock from re-running if foo changes. If foo changes, live.text handles updating the DOM itself. There's no need to rerun ifBlock.

Proposal

The idea is to:

  1. make an observation see any observables created within its function and to be able to tear them down automatically when the function returns.
  2. schedule any child observations "after" the parent observation, allowing the parent to run first.

In the example above, if ifBlock were to re-run its function, both LOGIC and foo should be torn down.

By torn down we mean all activity should stop. Any handlers listening to those observables should be removed (watch out for handlers bound with .listenTo()). Also those observables should not be binding on their dependencies. Currently, I believe can-event-queues/value supports calling .off() without arguments to tear down any handlers. This should probably be formalized, possibly as a canReflect.stop() (which could also work for Map and List types).

Finally, the child-parent relationship needs to be established. I think part of this can happen with expansion of can-observation-recorder. An observation record could contain a set of createdDependencies:

createdDependencies: new Set()

I think child observables can register themselves with:

ObservationRecorder.created( observable )

When can-observation updates its value, it can go through createdDependencies and call canReflect.stop(observable) on each observable.

Other

  • As observation's will take on their "parent's" priority + 1, Infinity + 1 is still Infinity. This might mean resuming the default priority of 0.
@matthewp
Copy link
Contributor

matthewp commented Jan 3, 2018

The thing that's fuzzy to me is if we should tear down the observations if something else is using it. Or if it would be possible for something else to be using that observation.

@justinbmeyer
Copy link
Contributor Author

The thing that's fuzzy to me is if we should tear down the observations if something else is using it. Or if it would be possible for something else to be using that observation.

I think it's extremely unlikely that something else is using the other observations but SHOULD NOT be torndown when the outer observation is re-calculated.

@matthewp
Copy link
Contributor

The way this is implemented makes an assumption that any observations created within another observation will always be recreated whenever the parent observation is updated.

This is not always the case, for example when the child observation is cached. This is the case with can-stache, which caches observations it creates for perf reasons. See here

@matthewp
Copy link
Contributor

I'm trying a slightly different approach than what is in #131. Instead of destroying all children whenever an observation is updated, will instead provide an API: trapBindings that traps calls to can.onValue. Pseudocode or how that will be used:

function fn() {
    // priority 2
    var foo = new Observation(function foo(){
        var val = bar.get();
        fooUpdates++;
        return val;
    }, {isObservable: false});

    var stopTrap = foo.trapBindings()
    Bindings.start()
    view.live(foo);
    ObservationRecorder.addBindings( stopTrap() )

    
    return foo.get();
}

@matthewp
Copy link
Contributor

We are going to defer doing this change (and removing can-view-nodelist) for now. I wrote up this gist which describes where I got with the task and what the next steps would be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants