Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

feat(parse): ability to bind once an expressions #7486

Closed
wants to merge 1 commit into from

Conversation

lgalfaso
Copy link
Contributor

Expressions that start with :: will be binded once. The rule
that binding follows is that the binding will take the first
not-undefined value at the end of a $digest cycle.

Watchers from $watch, $watchCollection and $watchGroup will
automatically stop watching when the expression(s) are bind-once
and fulfill.

Watchers from text and attributes interpolations will
automatically stop watching when the expressions are fulfill.

All directives that use $parse for expressions will automatically
work with bind-once expressions as long as they are evaluated
over a $scope. E.g.

<div ng-bind="::foo"></div>
<li ng-repeat="item in ::items">{{::item.name}};</li>

Have fun

benchmark: http://plnkr.co/edit/rJOqa0oh35Y0xBhTYZ6w?p=info
design doc: https://docs.google.com/document/d/1fTqaaQYD2QE1rz-OywvRKFSpZirbWUPsnfaZaMq8fWI/edit#

@mary-poppins
Copy link

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

  • Uses the issue template (#7486)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

function result(self, locals) {
if (!stable) {
lastValue = expression(self, locals);
result.$$unwatch = isDefined(lastValue) && '$$unwatch';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the purpose of using the '$$unwatch' string literal as the value? why not just simple true?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the chances of seeing an '$$unwatch' are smaller than true and will allow us to reuse the same property in the future

@joshkurz
Copy link
Contributor

are there any plans to set up some official performance tests for this feature? It would be nice to see some tangible benefits. I know this is faster theoretically, but maybe having some benchmarks would be nice IMO.

@IgorMinar
Copy link
Contributor

@joshkurz I don't think that making it configurable is that useful (unlike the interpolation markers). Everything inside of our interpolation markers should not be interpreted by anybody else, so you can think of it as an special operator in our expressions.

@joshkurz
Copy link
Contributor

@IgorMinar that makes perfect sense.

On Fri, May 16, 2014 at 4:22 PM, Igor Minar notifications@git.luolix.topwrote:

@joshkurz https://github.com/joshkurz I don't think that making it
configurable is that useful (unlike the interpolation markers). Everything
inside of our interpolation markers should not be interpreted by anybody
else, so you can think of it as an special operator in our expressions.


Reply to this email directly or view it on GitHubhttps://github.com//pull/7486#issuecomment-43375243
.

Josh Kurz
www.joshkurz.net

One-time binding expressions will retain the value of the expression at the end of the
digest cycle as long as that value is not undefined. If the value of the expression is set
within the digest loop and later, within the same digest loop, it is sent to undefined,
then the expression is not fulfill and will remain watched.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: "fulfilled"

@IgorMinar
Copy link
Contributor

I mentioned earlier that we should consider throwing an error if one-time binding are used with ngModel. Since then I realized that there are some use-cases when you want to display a value via a possibly read-only input field and you want the value to be formatted, so I think we should allow it. So ignore my previous comment about throwing an error :)

@IgorMinar
Copy link
Contributor

@joshkurz here is a benchmark: http://plnkr.co/edit/rJOqa0oh35Y0xBhTYZ6w

@IgorMinar
Copy link
Contributor

btw the benchmark shows that our interpolation is 2x slower than direct binding via ngBind. 2x seems too much, so we should investigate why that's the case. but that's for another PR/issue

@lgalfaso lgalfaso added cla: yes and removed cla: no labels May 19, 2014
@@ -1246,13 +1246,19 @@ function $ParseProvider() {
};

return function(exp) {
var parsedExpression;
var parsedExpression,
oneTime;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know how much we care, but by the Google style guide, oneTime should be indented 4 rather than 2 spaces.

Expressions that start with `::` will be binded once. The rule
that binding follows is that the binding will take the first
not-undefined value at the end of a $digest cycle.

Watchers from $watch, $watchCollection and $watchGroup will
automatically stop watching when the expression(s) are bind-once
and fulfill.

Watchers from text and attributes interpolations will
automatically stop watching when the expressions are fulfill.

All directives that use $parse for expressions will automatically
work with bind-once expressions. E.g.

<div ng-bind="::foo"></div>
<li ng-repeat="item in ::items">{{::item.name}};</li>
@ermalmino
Copy link

👍

@lgalfaso lgalfaso closed this in cee429f May 23, 2014
RichardLitt pushed a commit to RichardLitt/angular.js that referenced this pull request May 24, 2014
Expressions that start with `::` will be binded once. The rule
that binding follows is that the binding will take the first
not-undefined value at the end of a $digest cycle.

Watchers from $watch, $watchCollection and $watchGroup will
automatically stop watching when the expression(s) are bind-once
and fulfill.

Watchers from text and attributes interpolations will
automatically stop watching when the expressions are fulfill.

All directives that use $parse for expressions will automatically
work with bind-once expressions. E.g.

<div ng-bind="::foo"></div>
<li ng-repeat="item in ::items">{{::item.name}};</li>

Paired with: Caitlin and Igor
Design doc: https://docs.google.com/document/d/1fTqaaQYD2QE1rz-OywvRKFSpZirbWUPsnfaZaMq8fWI/edit#
Closes angular#7486
Closes angular#5408
@jimmywarting
Copy link
Contributor

Heven't done any benchmark but i tried the plnkr example. (Apart from the number of watcher and digest cycle benifit) It felt like the first bind/interpolate-once initialize took way longer then the first baseline-binding/interpolation initialization. (around 7-8 sec)
brings a couple of question:

  1. Is it only me? can anyone else confirm this?
  2. why is that?
  3. what can we do to improve it?

@Cinamonas
Copy link

This is a very anticipated addition, but I encountered an issue where {{ ::foo }} fails with syntax error:

Syntax Error: Token ':' not a primary expression at column 2 of the expression

while {{::foo }} works correctly.

I believe this was not an intended behaviour?

@lgalfaso
Copy link
Contributor Author

lgalfaso commented Jun 3, 2014

@Cinamonas #7640 took care of that, it should be part of the next beta

@bolasblack
Copy link
Contributor

Wow ! Nice !

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

Successfully merging this pull request may close these issues.

10 participants