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

Knockout should only update bindings when their dependencies change #321

Closed
mbest opened this issue Feb 9, 2012 · 12 comments
Closed

Knockout should only update bindings when their dependencies change #321

mbest opened this issue Feb 9, 2012 · 12 comments
Assignees
Milestone

Comments

@mbest
Copy link
Member

mbest commented Feb 9, 2012

An element can include multiple bindings. Often an element will include a combination of lightweight and heavyweight bindings. The former include visible, css, enable, attr. The latter include template, options, html. Currently the dependencies of all bindings for an element are linked; thus an update to the dependency for one binding will update all bindings. This negatively affects performance when only a lightweight binding needs updating, but the heavyweight binding is also updated.

I propose that Knockout's binding system be changed so that bindings are independent of each other. This fits with one of Knockout's main features, that it “automatically updates the right parts of your UI whenever your data model changes.” (Knockout: Introduction)

Unfortunately this is a breaking change because many people have written code that depends on the current, non-independent nature of the binding system. For example, they may have a custom binding that doesn't set up its own dependencies; instead, it assumes it will be updated when another binding for the same element is updated.

Until this issue is fixed in Knockout, there are a few workarounds to help with performance. Bindings can make themselves run independently (mostly) of other bindings by, within their init function, creating their own computed observable to wrap their actions or by using the subscribe function to control updates. See the async binding I created as an example of the former. Within Knockout itself, the template binding also does this (but it's less independent since it's within its update function). Another option is to create an extra nesting layer to separate the bindings. For example, instead of combining the visible and template bindings on the same element, the visible binding can be placed on a div element enclosing the template element.

@mbest
Copy link
Member Author

mbest commented Feb 9, 2012

I've actually already created two different updates to Knockout that implement independent bindings, async_bindings (#221) and smart-binding (#315). My latest one includes a few other fixes and features, but at some point, I'll extract the code to support independent bindings and attach it to this issue.

@andrewharry
Copy link

I think this sounds like a brilliant change. Perhaps it could be something that is toggled on (off by default)

@mbest
Copy link
Member Author

mbest commented Feb 16, 2012

Thanks, Andrew. At first, it will have to be toggled on. But since it's the more correct behavior, at some point it should become default (with an option to turn it off) .

@UncleGringo
Copy link

Thanks for explaining this. It really appeared to be a bug. But now that I see that it's left over to not break stuff I get it. Is this behavior documented in the main docs? It really can have some serious performance issues that people should be aware of.

@mbest
Copy link
Member Author

mbest commented May 3, 2012

In order to fully fix this problem, we need to understand and analyze all the ways a binding can have a dependency and determine which of those dependencies are correct and which aren't. Here's what I believe is a complete list of dependency possibilities in the current version of Knockout:

  1. A top-level, observable view model (passed to applyBindings) becomes a dependency of all top-level bindings (those not part of a child context). Updating the view model observable will update all the top-level bindings.
  2. Observables accessed during the parsing of an element's binding become a dependency of all bindings for that element. Updates to those observables cause the binding to be re-parsed and all bindings to be updated.
  3. Observables accessed by a binding's init function become a dependency of all bindings for that element. But they only can cause one update since they cease to be dependencies after the first update.
  4. Observables accessed by a binding's update function become a dependency of all bindings for that element. Updates to those observables cause the binding to be re-parsed and all bindings to be updated.

Here's a jsfiddle example showing all of the above possibilities.

@mbest
Copy link
Member Author

mbest commented May 3, 2012

This is what I think the dependencies should be in the above cases:

  1. A top-level observable view model should be a dependency for all bindings, including in child contexts.
  2. No observables should be accessed during parsing of an element's binding. If the binding includes expressions that would access an observable, those expressions should be wrapped in a function that can be called within the context of the specific binding.
  3. Observables accessed in an init function should not create any dependencies.
  4. Observables accessed in an update function should only be dependencies of the binding in which they were accessed.

I've updated the example to use independent bindings using my smart-binding fork, which shows the behavior according to the above list. (I made a couple of minor changes to the example code to make it work with the independent bindings setting.)

For comparison, here is the example using my fork but without the independent binding option. The only difference from the current Knockout version is for item 1, that bindings in child contexts are updated when the top-level view model is updated.

@mbest
Copy link
Member Author

mbest commented May 3, 2012

Although I've already made an update to Knockout to support independent bindings, I plan to port those changes (or similar) in stages. Here are the stages I envision:

  1. Enhancement to computed observables: can watch multiple nodes for disposal (computed can be attached to multiple nodes and dispose when all are removed #484), can optionally return the computed value instead of the computed observable if it has no dependencies (binding system should only use computed when there are dependencies #483), can allow access to observables in read function without creating a dependency (Should be able to read an observable without creating a subscription #326).
  2. Binding contexts directly handle observable view models. Child contexts are updated if parent is updated. Bindings are re-parsed and updated if context is updated (support observable view models at both the root and child context level #485).
  3. Modify binding parser to support enhanced preprocessing (?). Add support for "x.y" syntax for two-level bindings, such as attr and css, allowing each sub-binding to have separate dependencies (consistent organization of bindings #226).
  4. Add options object to applyBindings to support optionally using independent bindings. Options will be stored in the binding context so that all bindings have access to them.
  5. Add support for independent binding dependencies: when option is set, wrap binding values in parser, unwrap values within each binding's value accessor function, and use a separate computed observable for each binding.

Update 6/22/12:

I've been thinking lately that it might be good to implement the independent bindings option as a binding provider. This would eliminate the need for item 4, but would require expansion of the binding provider interface to allow it to control how bindings are run.

@mbest
Copy link
Member Author

mbest commented Jun 28, 2012

I've created issue #546 that would move the binding processing into an extensible system. This would allow us to implement the functionality described in this issue as an extension to the binding system that can be released as a plugin at first. The next step, I think, would be to include it as a second binding processor within Knockout (similar to how there are two template engines).

If we decide to not implement #546 (or do it later), then I would prefer to go ahead with implementing this using something similar to how I did it in my fork (with an option to applyBindings).

@mbest
Copy link
Member Author

mbest commented Jul 31, 2012

It looks like this might be better done, as Steve suggested in #422, in an upcoming version 3.0. That way we don't have to worry as much about the compatibility issues.

For version 2.2, #500 is a good compromise as it allows custom binding handlers to be set up to update independently, and makes it easy to use a simple pattern to make built-in bindings independent as well.

@mbest
Copy link
Member Author

mbest commented Sep 21, 2012

One related problem that I've seen reported recently is the combination of the value binding with something else such as enable or hasfocus. Because the value binding doesn't update the model right away, the view and model will be out of sync for a time. If the other binding is updated before the value binding updates the model, the value binding will also be updated and overwrite the view from model.

@mbest
Copy link
Member Author

mbest commented Jan 25, 2013

Here's a related issue that came up the forums: https://groups.google.com/d/topic/knockoutjs/ROyhN7T2WJw/discussion

Example: http://jsfiddle.net/aburkholder/RnJ2R/1/

If the update handler of one binding updates an observable that should update a sibling binding, the sibling binding won't get updated. Incidentally, this scenario worked back in version 2.0.0 because that version allowed computed observables to update themselves.

@mbest
Copy link
Member Author

mbest commented May 7, 2013

Completed with #946.

@mbest mbest closed this as completed May 7, 2013
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

3 participants