This repository was 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.4k
feat($compile): add one-way binding to the isolate scope defintion #13928
Closed
Closed
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
31d31e4
feat($compile): add one-way binding to the isolate scope defintion
Narretz b7389f5
fixes
Narretz 35f5b5b
update test expectation
Narretz e141438
deep-watch object literals, fix typos
Narretz 368ce25
remove shallow watch, update docs
Narretz e5db80d
fix typos, wordings
Narretz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -186,6 +186,28 @@ | |
* you want to shallow watch for changes (i.e. $watchCollection instead of $watch) you can use | ||
* `=*` or `=*attr` (`=*?` or `=*?attr` if the property is optional). | ||
* | ||
* * `<` or `<attr` - set up one-way (one-directional) binding between a local scope property and the | ||
* parent scope property of name defined via the value of the `attr` attribute. If no `attr` | ||
* name is specified then the attribute name is assumed to be the same as the local name. | ||
* Given `<dir my-attr="parentModel">` and directive definition of | ||
* `scope: { localModel:'<myAttr' }`, then isolate scope property `localModel` will reflect the | ||
* value of `parentModel` on the parent scope. Any changes to `parentModel` will be reflected | ||
* in `localModel`, but changes in `localModel` will not reflect in `parentModel`. There are however | ||
* two caveats: | ||
* 1. one-way binding does not copy the value from the parent to the isolate scope, it simply | ||
* sets the same value. That means if your bound value is an object, changes to its properties | ||
* in the isolate scope will be reflected in the parent scope. | ||
* 2. one-way binding watches changes to the **identity** of the parent value. That is important should | ||
* you one-way bind an object, and then replace that object in the isolated scope. If you now change | ||
* a property of the object in your parent scope, the change will not be propagated to the isolated | ||
* scope, because the identity of the object has not changed. Instead you must assign a new object. | ||
* | ||
* One-way binding is useful if you do not plan to propagate changes to your isolated scope bindings | ||
* back to the parent. However, it does not make this completely impossible. | ||
* | ||
* Same as with bi-directional bindings, you can also use shallow watch for changes (i.e. $watchCollection instead of $watch): | ||
* `<*` or `<*attr` (`<*?` or `<*?attr` if the property is optional). | ||
* | ||
* * `&` or `&attr` - provides a way to execute an expression in the context of the parent scope. | ||
* If no `attr` name is specified then the attribute name is assumed to be the same as the | ||
* local name. Given `<widget my-attr="count = count + value">` and widget definition of | ||
|
@@ -826,7 +848,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { | |
var EVENT_HANDLER_ATTR_REGEXP = /^(on[a-z]+|formaction)$/; | ||
|
||
function parseIsolateBindings(scope, directiveName, isController) { | ||
var LOCAL_REGEXP = /^\s*([@&]|=(\*?))(\??)\s*(\w*)\s*$/; | ||
var LOCAL_REGEXP = /^\s*([@&]|[=<](\*?))(\??)\s*(\w*)\s*$/; | ||
|
||
var bindings = {}; | ||
|
||
|
@@ -2962,7 +2984,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { | |
optional = definition.optional, | ||
mode = definition.mode, // @, =, or & | ||
lastValue, | ||
parentGet, parentSet, compare; | ||
parentGet, parentSet, compare, removeWatch; | ||
|
||
switch (mode) { | ||
|
||
|
@@ -3023,7 +3045,6 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { | |
return lastValue = parentValue; | ||
}; | ||
parentValueWatch.$stateful = true; | ||
var removeWatch; | ||
if (definition.collection) { | ||
removeWatch = scope.$watchCollection(attrs[attrName], parentValueWatch); | ||
} else { | ||
|
@@ -3032,6 +3053,29 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { | |
removeWatchCollection.push(removeWatch); | ||
break; | ||
|
||
case '<': | ||
if (!hasOwnProperty.call(attrs, attrName)) { | ||
if (optional) break; | ||
attrs[attrName] = void 0; | ||
} | ||
if (optional && !attrs[attrName]) break; | ||
|
||
parentGet = $parse(attrs[attrName]); | ||
|
||
destination[scopeName] = parentGet(scope); | ||
|
||
if (definition.collection) { | ||
removeWatch = scope.$watchCollection(attrs[attrName], function onParentCollectionValueChange(newParentValue) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can reuse There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It works, but I wonder if $parse isn't doing extra work? Or does it detect an already parsed expression? |
||
destination[scopeName] = newParentValue; | ||
}); | ||
} else { | ||
removeWatch = scope.$watch(attrs[attrName], function onParentValueChange(newParentValue) { | ||
destination[scopeName] = newParentValue; | ||
}); | ||
} | ||
removeWatchCollection.push(removeWatch); | ||
break; | ||
|
||
case '&': | ||
// Don't assign Object.prototype method to scope | ||
parentGet = attrs.hasOwnProperty(attrName) ? $parse(attrs[attrName]) : noop; | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 think the character should be
>
giving more feeling of forward movementThere 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 I wrote in #13854:
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.
Trouble is that in my mind
<attrName
looks like a broken HTML elementThere 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.
How about
<=
? It has the same connatation of moving from attribute to property (even more so) it has an element of=
about it and looks nicepropertyName: '<=attrName'
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.
Having this connotation is not as important as not having the opposite. As for multi-character sigils, I think they would look too scary in combination with
?
and*
: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 happy to go with the consensus and get this in :-)
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.
What was the reason
-
didn't make sense?bindings: { propertyName: "-*?attrName" }