Skip to content
This repository has been archived by the owner on Mar 13, 2018. It is now read-only.

Unbinding a model does not completely make the template's contents disappear #128

Closed
jmesserly opened this issue Jul 25, 2013 · 17 comments
Closed

Comments

@jmesserly
Copy link
Contributor

(original bug https://code.google.com/p/dart/issues/detail?id=11983)

I took @sethladd 's example and integrated it into MDV's sample.html:

<!DOCTYPE html>
<html>
  <head>
    <script src="mdv.js"></script>
  </head>
  <body>
    <p>
      The below will appear and disappear as a model object is
      bound and unbound from the template. Conditionals are implemented
      simply as "this template appears if something is bound" and
      "this template disappears if nothing is bound".
    </p>

    <template id="greeting" bind>
      <p>Hello {{msg}}</p>
    </template>
  <script>
  document.addEventListener('DOMContentLoaded', function() {
    var t = document.getElementById('greeting');
    var model = { msg: 'world' };
    t.model = model;

    // Needed to detect model changes if Object.observe
    // is not available in the JS VM.
    Platform.performMicrotaskCheckpoint();

    setInterval(function() {
      if (!t.model) {
        t.model = model;
      } else {
        t.model = void 0;
      }
      Platform.performMicrotaskCheckpoint();
    }, 1000);

  });
  </script>
  <body>
</html>
@jmesserly
Copy link
Contributor Author

btw, this has the same results with either null or undefined

@sethladd
Copy link

Thanks for filing.

@jmesserly
Copy link
Contributor Author

hmm, thinking about this some more, I think you need to add an if to the template. However, when I do that, it still expands the template, which is not what I'd expect. Now I'm confused :)

I did get it to work by adding if="{{}}" ... this makes me wonder if if should work like bind and repeat, where an empty attribute implies '{{}}' ?

Here is the working version:

<!DOCTYPE html>
<html>
  <head>
    <script src="mdv.js"></script>
  </head>
  <body>
    <p>
      The below will appear and disappear as a model object is
      bound and unbound from the template. Conditionals are implemented
      simply as "this template appears if something is bound" and
      "this template disappears if nothing is bound".
    </p>

    <template id="greeting" if='{{}}'>
      <p>Hello {{msg}}</p>
    </template>
  <script>
  document.addEventListener('DOMContentLoaded', function() {
    var t = document.getElementById('greeting');
    var model = { msg: 'world' };
    t.model = model;

    // Needed to detect model changes if Object.observe
    // is not available in the JS VM.
    Platform.performMicrotaskCheckpoint();

    setInterval(function() {
      if (!t.model) {
        t.model = model;
      } else {
        t.model = void 0;
      }
      Platform.performMicrotaskCheckpoint();
    }, 1000);

  });
  </script>
  </body>
</html>

@sethladd
Copy link

Thanks for researching this, John. I asked more about what bind means, and if this is really a bug, on this thread: https://groups.google.com/forum/?fromgroups=#!topic/polymer-dev/nBdPaVMtcys

if="{{}}" and just if seem redundant if bind means "this template should activate and insert into the document if a model is bound". There's already an implicit if in there. But I might not understand the semantics of bind.

@jmesserly
Copy link
Contributor Author

There's already an implicit if in there. But I might not understand the semantics of bind.

That's just the thing, there isn't an implicit "if". "If" implies "bind", not the other way around. "bind" just means "make this template active using the provided model". You can also use it to get at a field, e.g. <template bind="{{foo.bar.baz}}">the value of foo.bar.baz.qux is: {{qux}}</template>

@jmesserly
Copy link
Contributor Author

You can think of bind as a 1-element repeat. (Which is exactly how it's implemented)

@sethladd
Copy link

Thanks John. This is clear to me: "make this template active using the provided model". However, shouldn't the opposite be true? ""make this template inactive if there is no model" That is how it works when the page initially starts up, before I ever bind to that template.

Maybe.... I'm "unbinding" by setting model to null (in my Dart version). Perhaps null isn't enough to fully and truly unbind?

@jmesserly
Copy link
Contributor Author

right, I don't think there is a way to "unbind" a template, unless it has the "if" attribute.
"null" is generally a valid model value.

It is rather strange that this has a side effect which cannot be reversed:

var node = querySelector('template'); // note: template has "bind" attribute
node.model = node.model; // set property to itself--harmless right?

you can reverse it in a few different ways:

node.unbindAll(); // option 1
node.unbind('bind'); // option 2
node.bind('if', false, ''); // option 3

None of those are very satisfying. Also I think MDV will leak a little bit of memory even with unbindAll (IIRC, the TemplateIterator internal object isn't freed when all of the TemplateBinding objects are unregistered).

@sethladd
Copy link

So that begs the question, how can you ask if a template has something that is bound? I was doing template.model == null but I guess that's a valid binding.

@jmesserly
Copy link
Contributor Author

Hmm, it should be possible to do via template.bindings.length == 0. I'm not sure if that is intended or not, but there is now a "bindings" Map exposed on Node (see Dart's Node.bindings).

In JavaScript it's probably safer to do if (template.bindings) since it can be undefined as well.

@rafaelw
Copy link
Contributor

rafaelw commented Aug 1, 2013

So, I agree with you guys that the current semantics are a bit weird. I'm at a loss as to what the "right" behavior is.

One option is that template instances will never be created if the model is undefined.

That would have the effect that <template bind>, template.model = undefined will always result in zero instances.

However, to be consistent with that, I think the needs to apply to repeat, so

<template repeat>, template.model = new Array(10)

would also produce zero instances (which pretty much everyone agrees is wrong).

This brings me to the fact that template instances must tolerate any model value.

FWIW, the real way to remove a "bound" instance is to unbind it, e.g.

template.unbind('bind').

That will remove the instance regardless (always).

template.model =

is really just a short hand for

  1. look at the attributes of the template,
  2. call bind() on each with the model value.

If you guys can think of a better way to express this API, I'm all ears.

@sethladd
Copy link

sethladd commented Aug 6, 2013

Thanks for the comments. I'm not sold that template instances must tolerate undefined or null as acceptable model values.

If template.model = foo calls bind() on each attribute of the template, then I would expect template.model = null to call unbind() on each attribute of the template.

It's weird to let me call .model = foo but then to undo that I need to call .unbind('bind'). It's not analogous.

@sethladd
Copy link

sethladd commented Aug 6, 2013

Raf, can you confirm that "FWIW, the real way to remove a "bound" instance is to unbind it, e.g." also implies that the template is removed from the DOM?

@rafaelw
Copy link
Contributor

rafaelw commented Aug 6, 2013

Seth: Yes.

@rafaelw
Copy link
Contributor

rafaelw commented Aug 6, 2013

Ok. So here's my proposal: add a clear() method to template which has the effect of removing all instance (by way of calling unbind('bind'), unbind('if') and unbind('repeat') on itself.

@jmesserly
Copy link
Contributor Author

Sounds reasonable to me.

@rafaelw
Copy link
Contributor

rafaelw commented Aug 6, 2013

clear() implemented here: 60d98f1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants