Skip to content

Commit

Permalink
Fix with-binding dependency re knockout/knockout#2285
Browse files Browse the repository at this point in the history
  • Loading branch information
brianmhunt committed Sep 1, 2017
1 parent 169d287 commit 8ee9b7d
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ For TODO between alpha and release, see https://github.com/knockout/tko/issues/1

## 🚜 Alpha-4 (ongoing)

* (with binding) Fix dependency count for function-arguments [knockout/knockout#2285]
* (options) Allow importing `ko` in node

## 🏰 Alpha-3 (30 June 2017)
Expand Down
4 changes: 2 additions & 2 deletions dist/ko.js
Git LFS file not shown
4 changes: 2 additions & 2 deletions dist/ko.js.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions dist/ko.min.js
Git LFS file not shown
4 changes: 2 additions & 2 deletions dist/ko.min.js.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 30 additions & 2 deletions packages/tko.binding.if/spec/withBehaviors.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,16 @@ import {
} from 'tko.bind';

import {
observable
observable, observableArray
} from 'tko.observable';

import { DataBindProvider } from 'tko.provider.databind'
import { MultiProvider } from 'tko.provider.multi'
import { VirtualProvider } from 'tko.provider.virtual'

import {bindings as templateBindings} from '../index.js';
import {bindings as withBindings} from '../index.js';
import {bindings as coreBindings} from 'tko.binding.core';
import {bindings as templateBindings} from 'tko.binding.template'


import 'tko.utils/helpers/jasmine-13-helper.js';
Expand All @@ -29,6 +30,7 @@ describe('Binding: With', function() {
})
options.bindingProviderInstance = provider;
provider.bindingHandlers.set(coreBindings);
provider.bindingHandlers.set(withBindings);
provider.bindingHandlers.set(templateBindings);
});

Expand Down Expand Up @@ -224,4 +226,30 @@ describe('Binding: With', function() {
item('three');
expect(testNode.childNodes[0]).toHaveValues(['three']);
});

it('should refresh on dependency update binding', function () {
// Note:
// - knockout/knockout#2285
// - http://jsfiddle.net/g15jphta/6/
testNode.innerHTML = `<!-- ko template: {foreach: items} -->
<div data-bind="text: x"></div>
<div data-bind="with: $root.getTotal.bind($data)">
Total: <div data-bind="text: $data"></div>
</div>
<!-- /ko -->`

function ViewModel () {
var self = this;
self.items = observableArray([{ x: observable(4) }])
self.getTotal = function() {
return self.items()
.reduce(function (sum, value) { return sum + value.x() }, 0)
}
}

var model = new ViewModel();
applyBindings(model, testNode);

model.items.push({ x: observable(15) })
})
});
13 changes: 10 additions & 3 deletions packages/tko.binding.if/src/ifIfnotWith.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,16 @@ class ConditionalBindingHandler extends AsyncBindingHandler {
}
get shouldDisplayIf () { return !!unwrap(this.value) }

getIfElseNodes () {
if (this.ifElseNodes) { return this.ifElseNodes }
if (dependencyDetection.getDependenciesCount() || this.hasElse) {
return cloneIfElseNodes(this.$element, this.hasElse)
}
}

render () {
// Create any dependencies on values created in value-functions.
if (typeof this.value === 'function') { this.value() }
const isFirstRender = !this.ifElseNodes
let shouldDisplayIf = this.shouldDisplayIf
let needsRefresh = this.needsRefresh
Expand All @@ -108,9 +117,7 @@ class ConditionalBindingHandler extends AsyncBindingHandler {

if (!needsRefresh) { return }

if (isFirstRender && (dependencyDetection.getDependenciesCount() || this.hasElse)) {
this.ifElseNodes = cloneIfElseNodes(this.$element, this.hasElse)
}
this.ifElseNodes = this.getIfElseNodes()

if (shouldDisplayIf) {
if (!isFirstRender || this.hasElse) {
Expand Down

0 comments on commit 8ee9b7d

Please sign in to comment.