alexbihary/knockout-router — pull request #1 by rniemeyer (2014-03-20 04:09:36+00:00)
Title: ensure that router binding cleans up and is disposed properly and binds with context

Very cool project! I was looking at the `router` binding piece and noticed a couple of things that seem like they could be tweaked slightly.

1. The binding uses a computed, but does not pass the `disposeWhenNodeIsRemoved` option, so it would not get cleaned up properly. Additionally, if an observable was accessed outside of the computed via the valueAccessor, then it would be possible for the `update` function to be triggered again and create another computed. The `update` function is wrapped in a computed by KO already, so it should not need to be in an additional computed.

2. When calling `ko.applyBindingsToNode` with a control-flow binding (or event/click), the bindingContext should be passed as the third argument, so that it is available. This would mean that things like `$root`, `$parent` would still work.

3. It could be problematic in a long-running application to call `applyBindingsToNode` over and over on the main element, as the binding would not get cleaned up. Also, if the module/template options are defined as an observable, then this could cause multiple bindings on the element to trigger again.

The way that this pull request handles this third item is to add a comment as a child of the element and add the `template` or `module` binding to it. Then at the beginning of the `update` function the children of the element are always removed/cleaned up.

Really likes the look of this router, going to dig into the code some more and try it out.