This repository has been archived by the owner on Apr 12, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 27.5k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(*): lazy one-time binding support
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 #7486 Closes #5408
- Loading branch information
Showing
13 changed files
with
503 additions
and
18 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great merge!
eager to see it!
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yaaaaay!!!
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Best performance merge :)
This won't work with directives like ng-class etc, right?
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chinchang excellent question! +1
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RIP bindonce
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chinchang it works with the parser, so you can prepend any parsed expression with it --- However, in the case of
ng-class
, you can't sayng-class="{someClass: ::someExpr}"
, because the::
needs to be at the front of the expression, not on specific parts of an expression.Hope that clears that up for you. You could, however, say
ng-class="::{someClass: someExpr}"
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! Thanks.
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If i understand correctly, ng-class="::{someClass: someExpr}" will be bound on first digest cycle even if someExpr evaluates to undefined, so it may not work as expected. Would it not be possible to support the :: syntax nested inside an expression?
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure that it will evaluate to undefined as this feature is thought to be lazy one-time binding, at least i hope it doesn't:(
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If someExpr evaluates to undefined, the ng-class expression will evaluate to {someClass: undefined}, which is not undefined, so it will not be evaluated again.
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andershessellund even when it would not be a perfect solution, you can still do
ng-class="::someExp && {someClass: someExp}"
.The main goal of this feature is to have less watchers on an app and have faster digest cycles. Supporting expressions like
ng-class="{someClass: ::someExpr}"
would be odd as that would imply that the watch is oversomeExpr
and not over the entire expression, and not having the watch over the entire expression will adds other limitations tong-class
(this would be out of the scope of the original conversation).This is the first one-time binding mechanism that is part of the core, as more people start using it some refinements will be needed.
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't begin to express how relieved I am to see this getting done! Dealing with this through avoiding declarative syntax or utilizing a third party solution (as referenced in the design docs) made me seriously uneasy. With this I can make performant solutions without hacking and/or circumventing the core ideas of angular.
Somebody said that they weren't sure many people were using this for dart, but judging from the number of performance topics related to grids and lists of things found around the web, I feel pretty confident a lot of people will love this just as much as I do.
Thank you!
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed in #6354, I do think that we need some sort of way to reset some or all "bind-once" expressions that have been interpolated.
i18n is the most immediately obvious use-case, given that i18n-enabled apps tend to have a ton of interpolation statements that only need to be executed once, or under very rare circumstances (when a user toggles languages).
That being said, I could easily see over-use of bind-once (and #6354's suggestion of bind-once namespacing) quickly becoming an anti-pattern, as developers spend too much time chasing after small performance gains, or using the functionality as a crutch to unnecessarily put computationally-expensive functions in bind-once interpolation statements, which is more likely than not an improper separation of MVC concerns...
This is a great feature, but IMO, it needs to be used judiciously.
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this expected to work in the case of parsing functions so
is parsed every time a digest occurs, but
is only evaluated once?
I had a quick play and in both cases the function is fired every digest. I would suppose the only way to accomplish this is execute functions in the controller and put the resulting variable on scope, using it with ::?
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lytnus In both cases, the expression should be parsed once. In the later case, it should be evaluated in every digest until
someFunction()
returns something other thanundefined
(keep in mind that within a$digest
, the function can be called multiple times.If this is not working, please create a plunker that reproduces the error
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad, further testing seems to imply it works as expected :)
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really awesome, from performance perspective my dislike when using ng-repeat is now kinda gone. Really cool this feature is available. Thanks!
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like a much deserved feature. Unfortunately having issues with the bind not always working http://stackoverflow.com/questions/25658378/angularjs-1-3-one-time-binding-not-always-working
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@maruf89 I am not able to make it fail with the plunker posted. Can you create a new one that shows the behavior you mention?
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lgalfaso Yea I'll mess around some more.
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this support in filters? exp:
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thammin --- sort of. the filter needs to return
undefined
when it receives anundefined
value, otherwise it will cause the model to be watched again.I recall we were talking about making builtin filters do this, and it's possible that a followup patch (which renders all filters pure and therefore not evaluated unnecessarily) will make it essentially automatic
cee429f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@caitp Will try it later, Thanks!