One time binding does not work with expression bindings

[fenduru]

I am writing a directive that has an expression (&) binding, and I want my consumers to be able to pass a one time binding if they wish. For instance, both of the following uses should be valid:

<my-directive option="::someOption"></my-directive>
<my-directive option="someOption"></my-directive>

I want to do this because the consumer of my directive knows whether or not their expression is ever going to change, and would like them to be able to add this optimization.

The problem is that if I use scope.$watch to watch the option, the listener still runs multiple times:

{
  restrict: 'E',
  scope: {
    option: '&'
  },
  link: function( scope ) {
    scope.$watch( 'option()', function() {
      // may run multiple times even if the user provided the expression "::someoption"
    }
}

The workaround for this is to use scope.$parent.$watch, but this seems like it should not be the primary way of consuming expressions. I think that expression bindings should benefit from one time bindings automatically, without the directive-writer having to think about whether or not the consumer might have a valid use-case to use a one time binding.

I think reason this is happening is due to the fact that while the parsed expression has metadata indicating that it should only be run once, the & binding wraps that function in order to pre-bind it to the correct scope, but does not copy the relevant metadata.

Here is a plunker demonstrating the behavior

[dcherman]

I'm running into this as well; can someone take a look? Thanks!

[kentcdodds]

This is a misunderstanding of how one-time bindings and statement bindings work. They were not designed to function in this way.

If you want to get the performance optimization, you could do something like this:

<my-directive option="someStatement()"></my-directive>
<my-directive option="someStatement()" run-once></my-directive>

Then your DDO would look like this:

{
  restrict: 'E',
  scope: {
    option: '&'
  },
  link: function(scope, el, attrs) {
    var runOnce = attrs.hasOwnProperty('runOnce');
    var stopWatching = scope.$watch('option()', function(value) {
      if (angular.isDefined(value) {
        doSomethingWith(value);
        if (runOnce) {
          stopWatching(); // <-- that accomplishes the same thing
        }
      }
    });
  }
}

That's a little more work, but like I said, what you're suggesting is simply not how statement bindings (the &) and one-time bindings (the ::) work.

[dcherman]

@kentcdodds How were they designed to function then? What's the use case for one time bindings if not to do exactly what we're trying to do in the example directive? At the moment, one-time bindings are only useful in non-isolate scope directives where you can watch attrs.foo or manually parse the expression and use $scope.$parent.$watch which is absurd.

Your example also falls over in the event that any given directive has more than one binding that may be intended to be a one-time binding.

[fenduru]

@kentcdodds IMO having framework features work in harmony is very important.

They were not designed to function this way

I'm fully aware of the shortcomings of the current implementation, which is why I filed the issue. If this was a conscious design decision, I would be interested in hearing the rationale behind it.

I absolutely do not think that consumers of a particular directive should have to know about the implementation detail and think about "is this a place where I can use the one time syntax". If ng-if="::myOneTimeVar" works, then how is the consumer supposed to know that <isolate-scope-directive some-attr="::myOneTimeVar"> won't work? In fact, the situation is even worse by failing silently. The person consuming the directive very clearly indicated their intent to have the binding run once, yet it will run every digest cycle without warning.

[kentcdodds]

@dcherman, the example implementation is just an example of how you might accomplish it. If you needed multiple one-time statements like that, you could come up with something for that...

@fenduru,

I absolutely do not think that consumers of a particular directive should have to know about the implementation detail and think about "is this a place where I can use the one time syntax".

I totally agree with you. Which is why we have a brand new syntax in Angular 2.0, because the current implementation of the template syntax makes not being forced to know the directive implementation impossible.

The difference between ngIf and someAttr is that ngIf is an expression binding (with =) and someAttr is a statement binding (with &) (at least, that's what options was in your example above). The one-time binding only works for expressions, not statements.

[fenduru]

@kentcdodds again, I fully understand how it currently works, but disagree with how it currently works, hence the issue.

The & binding still calls $parse which knows all about ::. The only benefit that & provides me is avoiding to have to combine $parse with the parent scope manually. If I want to support ::, I still can, I just have to give up the convenience of &

WRT ngIf, that is not at all how it is implemented. The only reason ngIf doesn't run into the same issue is that it doesn't need an isolate scope.

In any of the possible ways of working around this bug (I consider something that we agree is wrong -- as indicated by your Angular 2 comment -- to be a bug, regardless of how it is implemented now), the directive developer has to make a conscious decision to support one-time bindings. But this is backwards, since very rarely does a directive know all of its consumers use cases and whether or not they require one time binding support. So as a library-author, I am left with a choice:

• Write convenient, idiomatic Angular directives and prevent consumers from choosing where to bind once
• Write inconvenient, unidiomatic Angular directives to support a framework feature (I want my framework to support me, not the other way around)

[fenduru]

Also, criticism without ideas is not very worthwhile, so my suggestion is to modify the function that $parse returns to look something like this (obviously this is high level pseudo code, and the actual implementation would take some thought):

function $parse( expression ) {
  var value,
        hasRun = false,
        oneTime = isOneTime( expression );

  return function() {
    if ( !oneTime || !hasRun ) {
        value = actuallyEvaluate( expression );
        hasRun = true;
    }
    return value;
}

[kentcdodds]

Now I understand what you mean. Thanks for clarifying. The pseudo code was helpful.

[lgalfaso]

Hi,
I will try to deconstruct all this as there are several aspects that need to be handled differently. I do not want to make this too long of a post, so I will put enough information for everybody to fill the gaps, but it will not be a complete explanation of everything talked here.

1. A one time binding expression is one that, if used in a $watch then: If at the end of a digest cycle the last value is not undefined then the watcher will get automatically unwatch

2. Within a single digest cycle, a watched expression can get called multiple times

3. A $parsed expression, if called multiple times, then the expression should get evaluated regardless of the fact that this is a one-time expression or not

    scope.$watch( 'option()', function() {
      ...
    });

will get called multiple times as the expression option() is not a one-time expression. If the idea is to use the value of the attribute option then it should be

    scope.$watch(attrs.option, function() {
      ...
    });

[rodyhaddad]

One-time binding is on bindings, not on expressions.

---

The issue with the pseudo-code provided is that one-time binding is, in practice, dependent on the scope its being watched on.

So a given one-time binding expression (e.g. ::expression), can be watched on multiple scopes at the same time, and might become stable in some scopes while still running in others (Think of one-time binding in an ng-repeat. Parsed once, watched multiple times)

So $parse can only do its one-time binding job correctly if it knows the $scope its being watched on.

For & to work, the original expression doesn't know what it's being watched on.

But aside from that, & is never meant to be watched. It works as a callback, you can pass it local variables, like a $event, and its great at doing its job.

Why are you watching it? Why not use = instead?

---

For your use case, I can't think of a reason why = wouldn't cut it.
Maybe you're trying to do something more elaborate? Don't hesitate to expand your example if you believe = isn't enough :)

Here's your plunkr with = instead of &.

[fenduru]

@lgalfaso Thank you for summarizing, but please see the original post/plunkr to see why scope.$watch is insufficient (unless you advocate scope.$parent.$watch, in which case why do we have & bindings).

The person consuming a directive is going to write:

<some-directive some-attr="::thisIsExpensivePleaseOnlyRunMeOnce">...

They have very nicely and declaritively indicated that they only wan't that to happen once (just like in the ngIf case where this does work). But now I, the author of some-directive need to have foresight to know that it is possible that my users may want some-attr to only run once, and I have to write my directive defensively. In fact, I may have many directives that were written pre-1.3 that would need to be re-written if I want my end-users to benefit from a feature added in Angular.

@rodyhaddad I understand the distinction between watchers/expressions, and the issues in my pseudo-code.

& is never meant to be watched

This is news to me, and find it hard to believe. = requires that you give it something assignable, so an expression like calculateFoo() or true is not allowed. Using & is extremely useful for 1-way binding, which is what I want in most of the directives I write. For instance, I am writing a calendar widget at the moment. It makes sense for the date attribute to be 2-way bound, but it does not make sense for something like minDate or maxDate to be 2-way bound since 1. the widget never writes to it, and 2. those can be calculated values.

I want to boil down the problem very simply:

• ngIf supports ::
• If ngIf had an isolate scope, then :: would break

Do you consider this correct? Are my consumers disallowed from using :: because I happen to have an isolate scope?

[jbedard]

@fenduru why not use = for 1-way binding? Expressions like calculateFoo(), true or a + b should work just fine as long as the isolated scope doesn't try to assign to them...

[gkalpak]

EDIT: I started writing this comment, when the last comment was #11416 (comment), so sorry if I seem to repeat what others have already said afterwards 😃

---

I still don't get it. One-time binding is a concept that applies (as the name suggests) to binding.
& does not set up a binding; it "provides a way to execute an expression in the context of the parent scope".

What you are trying to achieve is add support for indicating that the result of a function is considered stable once it is not undefined at the end of a $digest cycle (and thus the function does not need to be called any more), but that is a different thing than binding a parent scope expression to a local scope function (which is what & is does).

Basically, you are proposing a whole new symbol for binding a value to the result of a one-time watched expression executed in the context of the parent scope. Whether this new type of "binding" is common/useful enough to justify further complicating the DDO is debatable, but in any case I don't think it is anyhow related to &.

As already discussed above, there are several ways to achieve what you want, which is not idiomatic to Angular, but to your directive. Here are a couple more suggestions (both with their pros and cons; not necessarily better or worse than what's already been proposed):

1. Use a = binding which "understands" one-time.
   Pros:

   • Natively understands one-time binding.
   • Automatically executes the expression for you and re-assigns, so you don't need a $watch.

   Cons:

   • If you want your user (the developer) to be able to bind to function expressions (which are non-assignable), you need to ensure that you are not trying to assign back. Basically, you are setting up a bi-directional binding, but should only use the one direction (from parent to isolate), or else an error will be thrown. So this approach is more susceptible to accidental errors.

2. Use a & and (idiomatically to your directive) recognize a leading :: to set up a one-time watch.
   E.g.:

   var oneTime = attrs.expr.indexOf('::') === 0;
   var watchExpr = (oneTime ? '::' : '') + 'expr()';
   scope.$watch(watchExpr, ...);
   

   Pros:

   • Less susceptible to errors / More declarative.

   Cons:

   • Use of leading :: is idiomatic to your directive.
   • Needs more boilerplate code. (If used extensively, you could wrap it into a helper and keep the overhead minimal, of course.)

Demo of the above two approaches.

[rodyhaddad]

@fenduru as @jbdeboer and @gkalpak pointed out, you can still use = if the user expression isn't assignable, as long as your directive doesn't actually assign something to it. That's probably already the case if you were using & anyway.

For minDate and maxDate in a calendar component, I would use =.
That would also allow users to change their min/maxDate on the fly, and have it automatically update in the directive, without any extra code from our part (cool!). Plus one-time binding works as it should.

---

For a calendar component, I would only use & for events. Something like <my-calendar on-change="executeThis($extraData)" />.

Actually, instead of having a date attribute, I would make my calendar integrate with ngModel, so users would be able to use things like ngChange and ngModelOptions out of the box. Once that's done, my minDate and maxDate would be binded via =, and used as extra validation in the validation pipeline of ngModel.

<my-calendar ng-model="user.birthday" my-min-date="::getBirthOf('Jesus')" my-max-date="todayWillChangeAtMidnight()" ng-model-options="{updateOn: 'blur'}" />

Then your calendar will start having classes like ng-untouched, ng-invalid and ng-dirty. Plus people familiar with ngModel won't have any surprises. Neat.