docs($compile): refine explanation of `=`-bindings #13921

thorn0 · 2016-02-02T00:51:10Z

The current version of this paragraph is in many ways inaccurate and confusing.

gkalpak · 2016-02-02T11:58:53Z

src/ng/compile.js

- *   `=*` or `=*attr` (`=*?` or `=*?attr` if the property is optional).
+ * * `=` or `=attr` - set up a bidirectional binding between a local scope property and an expression
+ *   passed via the attribute `attr`. The expression is evaluated in the context of the parent scope and
+ *   can be either an assignable expression or a literal value. If no `attr` name is specified then the


Since you are at it, it might be a good idea so somehow mention that if a non-assignable expression is passed, then $compile:nonassign will be thrown when trying to assign to it.

gkalpak · 2016-02-02T17:32:45Z

src/ng/compile.js

+ *   attribute name is assumed to be the same as the local name. Given `<widget my-attr="parentModel">`
+ *   and the widget definition `scope: { localModel:'=myAttr' }`, the property `localModel` on the widget's
+ *   scope will reflect the value of `parentModel` on the parent scope. Changes to `parentModel` will be
+ *   reflected in `localModel` and vice versa. If the attribute doesn't exist, an exception will be thrown


Actually, I think it will only throw if you try to assign and the expression is non-assignable or the attribute is missing.

Right. I believe that's exactly what I wrote.

I'm not sure I was clear. I meant it will throw if:

The expression is non-assignable and you try to assign to it.

The attribut is missing and you try to assign to it.

I believe that the part in bold is missing from the description.

Ah, I see. You're right. But is it worth going into these details when all we need here is just to give a notion of optional bindings? Won't it overcomplicate the explanation?

However, I found another interesting detail missing from this text. Namely, when the expression is an object or array literal, the watch is deep.

I mean, if the attribute is optional, please mark it as such. The idea should be like this. It doesn't really matter how and when exactly it fails if you didn't mark it.

thorn0 · 2016-02-02T20:31:04Z

@gkalpak pls have a look

gkalpak · 2016-02-02T22:27:05Z

src/ng/compile.js

- *   `=*` or `=*attr` (`=*?` or `=*?attr` if the property is optional).
+ * * `=` or `=attr` - set up a bidirectional binding between a local scope property and an expression
+ *   passed via the attribute `attr`. The expression is evaluated in the context of the parent scope and
+ *   must be either an assignable expression or a literal value. If no `attr` name is specified then the


It is not required to be an assignable expression. Only if you try to assign to it :)

Narretz · 2016-02-03T10:30:04Z

Looks good. One suggestion, can you change all instances of widget to directive? I widget is a relict of very old times, iirc, back before the term directive was introduced.

Narretz · 2016-02-03T10:44:32Z

src/ng/compile.js

+ *   doesn't exist, an exception ({@link error/$compile/nonassign `$compile:nonassign`}) will be
+ *   thrown on discovering changes to the local value since it'll be impossible to sync it with the
+ *   parent scope in this case. By default, the {@link ng.$rootScope.Scope#$watch `$watch`} method is
+ *   used for tracking changes, and the equality check is done by reference. However, if an object


Personally, I find it clearer to say that the equality check is based on object identity. I guess it means the same though.

thorn0 · 2016-02-03T11:04:40Z

can you change all instances of widget to directive?

What about "component"? It's already called so a bit above:

{...} (an object hash): A new "isolate" scope is created for the directive's element. [...] This is useful when creating reusable components...

Narretz · 2016-02-03T11:15:38Z

I would be okay with <my-component my-attr="parentModel">, but directive definition and directive's scope. Component is a rather specific term. I think the part you quoted shows that. Components imply a certain modularity, but directives can also add events etc. without being components.

The current version of this paragraph is in many ways inaccurate and confusing.

thorn0 · 2016-02-03T11:33:54Z

OK. Eliminated widgets and replaced 'by reference' with 'based on object identity'.

But I also don't like that it says "local scope property", "property of the directive's scope" everywhere whereas the same binding mechanism is used for controllers.

Narretz · 2016-02-03T11:42:31Z

I think that's okay. The text is very information dense anyway. bindToController should make clear that it changes this behavior.

thorn0 · 2016-02-03T11:45:57Z

I think bindings should get a higher level section of their own, like Transclusion. It can be done later though.

gkalpak · 2016-02-03T12:29:18Z

LGTM

Narretz · 2016-02-03T12:41:41Z

@gkalpak Can you merge? Then I can rebase the one-way PR.

The current version of this paragraph is in many ways inaccurate and confusing. Closes #13921

gkalpak · 2016-02-03T13:02:15Z

@Narretz: Merged.
(Backported to v1.4.x as 8a1f600.)

googlebot added the cla: yes label Feb 2, 2016

thorn0 force-pushed the patch-10 branch 2 times, most recently from 1201468 to 893a273 Compare February 2, 2016 01:33

gkalpak reviewed Feb 2, 2016
View reviewed changes

gkalpak added type: docs component: $compile severity: confusing labels Feb 2, 2016

gkalpak added this to the Backlog milestone Feb 2, 2016

gkalpak self-assigned this Feb 2, 2016

thorn0 force-pushed the patch-10 branch from 893a273 to 787def2 Compare February 2, 2016 13:26

thorn0 mentioned this pull request Feb 2, 2016

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

Closed

gkalpak reviewed Feb 2, 2016
View reviewed changes

thorn0 force-pushed the patch-10 branch from 787def2 to e8e2375 Compare February 2, 2016 20:06

thorn0 force-pushed the patch-10 branch from e8e2375 to 574f25b Compare February 2, 2016 22:14

gkalpak reviewed Feb 2, 2016
View reviewed changes

thorn0 force-pushed the patch-10 branch 4 times, most recently from 5af6732 to b163540 Compare February 3, 2016 00:28

Narretz reviewed Feb 3, 2016
View reviewed changes

docs($compile): refine explanation of =-bindings

c3bb547

The current version of this paragraph is in many ways inaccurate and confusing.

thorn0 force-pushed the patch-10 branch from b163540 to c3bb547 Compare February 3, 2016 11:29

gkalpak referenced this pull request in Narretz/angular.js Feb 3, 2016

remove shallow watch, update docs

368ce25

gkalpak closed this in 507cf31 Feb 3, 2016

gkalpak pushed a commit that referenced this pull request Feb 3, 2016

docs($compile): refine explanation of isolate scope =-binding

8a1f600

The current version of this paragraph is in many ways inaccurate and confusing. Closes #13921

thorn0 deleted the patch-10 branch February 3, 2016 13:11

docs($compile): refine explanation of =-bindings #13921

docs($compile): refine explanation of =-bindings #13921

Uh oh!

Conversation

thorn0 commented Feb 2, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thorn0 commented Feb 2, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Narretz commented Feb 3, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thorn0 commented Feb 3, 2016

Uh oh!

Narretz commented Feb 3, 2016

Uh oh!

thorn0 commented Feb 3, 2016

Uh oh!

Narretz commented Feb 3, 2016

Uh oh!

thorn0 commented Feb 3, 2016

Uh oh!

gkalpak commented Feb 3, 2016

Uh oh!

Narretz commented Feb 3, 2016

Uh oh!

gkalpak commented Feb 3, 2016

Uh oh!

Uh oh!

docs($compile): refine explanation of `=`-bindings #13921

docs($compile): refine explanation of `=`-bindings #13921