jquery.tmpl interpolation of observables are not updated when inside data-bind'ed elements #739

Merged
merged 3 commits into from Dec 4, 2012

Projects

None yet

3 participants

@vangberg

I am in the process of upgrading from 1.2.1 to 2.2.0. I have found a slight change in behaviour, using jquery.tmpl interpolation inside elements with a data-bind used to work with 1.2.1, but any versions >= 2.0.0 doesn't. The following snippet reproduces the issue:

<html>
  <head>
    <script src="jquery-1.8.3.js"></script>
    <script src="jquery.tmpl.min.js"></script>
    <script src="knockout-2.2.0.js"></script>
  </head>
  <body>
    <input data-bind='value: users()[0].name' />

    <h2>Good</h2>
    <div data-bind='template: {name: "good-template", foreach: users}'></div>

    <h2>Bad</h2>
    <div data-bind='template: {name: "bad-template", foreach: users}'></div>

    <script id="good-template" type="text/x-jquery-tmpl">
      <div>
        ${name()}
      </div>
    </script>

    <script id="bad-template" type="text/x-jquery-tmpl">
      <div data-bind='click: function() {}'>
        ${name()}
      </div>
    </script>

    <script type="text/javascript">
      var user = {name: ko.observable()};
      var viewModel = {users: ko.observableArray()};
      viewModel.users.push(user);
      ko.applyBindings(viewModel);
    </script>
  </body>
</html>

If I change the template to this:

    <script id="bad-template" type="text/x-jquery-tmpl">
      ${name()}
      <div data-bind='click: function() {}'>
        ${name()}
      </div>
    </script>

it actually works, though. So it seems like the observable is not registered, somehow.

@vangberg

In my last template, even plain text makes it work again:

<script id="bad-template" type="text/x-jquery-tmpl">
  foo
  <div data-bind='click: function() {}'>
    ${name()}
  </div>
</script>
@SteveSanderson
Collaborator

Thanks for reporting this. It is a bug.

The underlying cause was that the array-to-dom-children mapping logic determines whether your mapping can be disposed by checking whether its first DOM node is still in the document. In the case where the first DOM node happens to be a memoization comment that gets stripped out during unmemoization, the entire foreach mapping gets disposed too early. I've added a pull request that changes the logic to check whether any node is still in the document, and not dispose the foreach mapping until they are all gone.

Michael - what do you think? Is this the solution you'd go for?

@mbest
Member
mbest commented Nov 30, 2012

Steve, this is a good change. I made basically the same change in my own Knockout fork. Here's the code I used:

return !ko.utils.arrayFirst(mappedNodes, ko.utils.domNodeIsAttachedToDocument);
@SteveSanderson
Collaborator

Good point. Updated the implementation of anyDomNodeIsAttachedToDocument.

@mbest mbest merged commit b22cd76 into master Dec 4, 2012
@mbest
Member
mbest commented Dec 4, 2012

Looks good. Merged.

@mbest mbest deleted the 739-foreach-with-template-rewriting-bugfix branch Jan 11, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment