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

can.Mustache: #if sections are rendered twice when activated from live-binding #477

Closed
bmomberger-reciprocity opened this issue Sep 14, 2013 · 10 comments
Assignees
Milestone

Comments

@bmomberger-reciprocity
Copy link
Contributor

http://jsfiddle.net/air_hadoken/V8VvF/2/

The mustache helper "bar" "foo" is called twice when the value of "quux" is flipped to true, as evidenced by the return value having two instances of "bar". When "baz" is called on initial render, it is only called once.

This happens because live binding on the section is triggered both by the "change" function on the attribute and on the attribute name event (not shown in the fiddle but it's what I concluded after investigating the problem). EDIT: I was wrong about this. Still investigating.

Suggestion: listen on the change event only.

@justinbmeyer
Copy link
Contributor

the compute created live-binding is listening to change? It should not be.

@ghost ghost assigned justinbmeyer Sep 14, 2013
@justinbmeyer
Copy link
Contributor

There isn't a "bar" helper?

@bmomberger-reciprocity
Copy link
Contributor Author

There isn't; that was supposed to be the "foo" helper.

@justinbmeyer
Copy link
Contributor

If you check out http://jsfiddle.net/V8VvF/3/ and open the console ... you see that "foo is called" twice total. I'm not seeing the issue. I'd expect it to be called 3 times if the problem you described was happening.

@bmomberger-reciprocity
Copy link
Contributor Author

the second "should be" line is calling "baz" instead, which is reading from a different var. there should be only one call to "foo" after obs.quux is updated.

@justinbmeyer
Copy link
Contributor

Sorry, I'm an idiot. It shouldn't have been called the first time.

@bmomberger-reciprocity
Copy link
Contributor Author

You're about the farthest thing from an idiot I can fathom. It wasn't called the first time, and I could have made that clearer. Here: http://jsfiddle.net/air_hadoken/V8VvF/4/

@justinbmeyer
Copy link
Contributor

Awwww Thanks! I confirmed this is a problem. This will be fixed in 1.2.

@justinbmeyer
Copy link
Contributor

Ok, I figured out what happens. When is evaluating:

{{#if quux}}

It looks up quux and then creates a compute out of it. That compute is then read. When the compute is read, live binding listens to the compute changing, but when the compute is read, it ends up calling obs.attr('quxx') which also sets up a live-binding on obs's quxx property.

So when quxx changes, it triggers a redraw, but that also changes the compute, which also triggers a redraw.

If you are reading a "bound" compute, it doesn't actually read from any child observes and just uses the cached value. But the compute is not bound yet, so the read goes through to the child observes.

There's two fixes that should happen and each one would fix it.

  1. A compute should use the same batchNum as the event that triggered them. This is a hacky fix, but one that will improve performance in general.
  2. I'm not sure about the other one. We can't really avoid creating computes for arguments because this is how the two-way binding helpers works.

Might it be possible to create a "capturing" compute. That you ignore any non-bound comptues? No because you might pass a non-bound compute to the template that gets read and you want to bind on it.

Maybe it's possible to make a temporary compute that gets ignored if it's read within the master compute? These computes will be created within can.Mustache ... if they are read within the primary compute's read ... the bindings get passed through. I think this will work!

(sorry for the random musings)

@justinbmeyer
Copy link
Contributor

Another, possibility is to automatically prevent indicating that computes created within another compute are read.

I'll put that another way ... the outer compute in:

var me = new can.Map({name: "Justin"});

var outer = can.compute(function(){

  var inner = can.compute(function(){
    return me.attr("name")
  })

  return inner() + " might have figured this out"
})

will not listen to changes in inner.

The problem with this might be comptues that are used many times. inner might be called many times. Binding on it, and forcing it to cache it's value could have performance improvements.

What's really needed is is a way to say "you will be bound to" to the inner compute and somehow prevent any reads within that.

Maybe if there is a __reading ... a compute being read should add itself to the ___reading, basically preventing other reads. Ah, I think this is it!

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

No branches or pull requests

2 participants