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

feat($compile): add one-way binding to the isolate scope defintion#13928

Closed
Narretz wants to merge 6 commits intoangular:masterfrom
Narretz:feat-compile-oneway
Closed

feat($compile): add one-way binding to the isolate scope defintion#13928
Narretz wants to merge 6 commits intoangular:masterfrom
Narretz:feat-compile-oneway

Conversation

@Narretz
Copy link
Copy Markdown
Contributor

@Narretz Narretz commented Feb 2, 2016

No description provided.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Couldn't parentGet be used here (to avoid double parsing) ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do we even need to parse this? We are just passing the string to watch, which parses this anyway. Yeah, I think I was confused about what's going oon in the two-way binding

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should parse it to init the child value. However, you can do this

scope.$watchCollection(parentGet, ...)

to not parse twice.

@Narretz Narretz force-pushed the feat-compile-oneway branch from bd1f619 to ca8cc76 Compare February 2, 2016 13:34
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Unecessary inject() (here and below) ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

They are necessary. (many other tests have them). I think it's because the $rootScope etc. "globals" are set up in a module'S return fn.

@Narretz Narretz force-pushed the feat-compile-oneway branch from ca8cc76 to 218b338 Compare February 2, 2016 14:14
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are the function names for the watch callbacks swapped?
Should't it be onParentValueChange here and onParentCollectionValueChange above?

@Narretz Narretz force-pushed the feat-compile-oneway branch from 218b338 to 93a857b Compare February 2, 2016 15:45
@Narretz Narretz force-pushed the feat-compile-oneway branch from 93a857b to 31d31e4 Compare February 2, 2016 15:51
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

OOC, what is the purpose of owOptref + owOptreference ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Came from a c&p'd test, but owOptreference doesn't look like it's needed.

@thorn0
Copy link
Copy Markdown
Contributor

thorn0 commented Feb 2, 2016

Currently, if an object literal or an array literal is passed as the expression for a two-way (=) binding, the set up watch is deep (uses angular.equals). The implementation in this PR doesn't do this with the new one-way bindings. IMO it'd more consistent if one-way bindings behaved exactly like two-way bindings, just without probability for the parent to get modified by them (with the known caveats).

@Narretz Narretz force-pushed the feat-compile-oneway branch from c0b6bdc to 35f5b5b Compare February 2, 2016 19:22
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I expected it to remain {name: 'b'}. Interesting...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Aha, parsed expression with inputs and $$watchDelegates. Nice 😃

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's why it works? I'm actually confused by this right now.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes. Parsing {name: name} the resulting parsed expression has inputs, so it has a $$watchDelegate that will re-evaluate the expression whenever the inputs change value (in this case, inputs is the name property on $rootScope).
So, whenever $rootScope.name changes, the expression will be evaluated and will return a new object, thus the change will be propagated to the child.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So this works, but when you are going deeper, you need objectEquality watch - I've added that right now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

X Tutup