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

updating exposed scope property within directive's link function cause exception in v1.4.9 #13937

Closed
avivr opened this issue Feb 3, 2016 · 10 comments

Comments

@avivr
Copy link

avivr commented Feb 3, 2016

Updating an exposed scope property within directive's link function cause exception in v1.4.9, with not in v1.4.8.
I couldn't find the relevant change in the changelog.

Here is a demo to show the problem:
https://jsfiddle.net/kkop339m/

Change the script tag to import v1.4.9 and run the code with console open.
You will see the following exception:

Error: [$compile:nonassign] Expression 'undefined' used with directive 'sampleDirective' is non-assignable!
http://errors.angularjs.org/1.4.9/$compile/nonassign?p0=undefined&p1=sampleDirective
at angular.js:68
at parentSet (angular.js:9092)
at parentValueWatch (angular.js:9105)
at Object.regularInterceptedExpression (angular.js:14646)
at Scope.$digest (angular.js:16087)
at Scope.$apply (angular.js:16359)
at bootstrapApply (angular.js:1680)
at Object.invoke (angular.js:4535)
at doBootstrap (angular.js:1678)
at bootstrap (angular.js:1698)

Same code runs in v1.4.8 with no errors.

@Narretz
Copy link
Contributor

Narretz commented Feb 3, 2016

We've fixed a regression with that in 1.4.9. It's expected that this throws, because you haven't marked your scope definition with ? to make it optional. It's not in the changelog, because the fix is in parse and not in compile: 4473b81

Here's the original issue: #13367

So it's not a regression, but it restores the behavior we actually want. The problem is that the original behavior is "lost" since 1.4, so for quite a while.

@avivr
Copy link
Author

avivr commented Feb 3, 2016

Thanks @Narretz !

Maybe a note about it in the "breaking changes" section would assist, because it cause one of my apps to break and it took a while to figure out the reason.

@Narretz
Copy link
Contributor

Narretz commented Feb 3, 2016

That's the thing, it's not technically a breaking change, because we fixed a regression.

@thetrevdev
Copy link

Regression or not. It was there for 9 versions over 8 months... This actually breaks applications that might have missed a parameter but continued to work. And its not even listed in the changelog...

@AckerApple
Copy link

SOOOOOOoooooooo, what this all spells out to me, is that I should just put a question mark after EVERY scope attribute, just to be safe and work like old angular 1.4.7 worked??????? This is whacky and hardly covered in any changelog.

@thetrevdev
Copy link

Don't get me wrong. I am a fan of fixing this behavior. However for us it is a breaking change as in some cases people missed marking properties as optional but treated it as such. However as a very large application it is impossible to know where we might have done this. For now I have upgraded to 1.4.9 / 1.5.0 but modified the angular source to treat it as a warning so we can fix without loss of functionality.

Basically reverted: 4473b81

@AckerApple
Copy link

Thank you for the reply. I've never needed nor ever used the optional attribute specification even though it seems to have existed since the beginning of Angular. And so for me, I can't see why we need an optional specifier at all, especially when my 1.4.6 code works so great. Specifying something as optional, just doesn't feel like JavaScript at all. But mass adding a ? to all me two-way binds =? fixed my code across the board without checking if any scope attribute actually needs it. Thanks again

@Narretz
Copy link
Contributor

Narretz commented Mar 8, 2016

The scope definition is a bit like an interface for your directive. So you can define explicitly to the consumer if a property must be set or can be set. To me that's a strength, not a weakness.

@AckerApple
Copy link

Narretz, a declaration of an optional attribute, is documentation and should not cause code to break. Yo, nice spin and healthy light on the subject but it was correct for how code should flow in older Angular versions. I should be able to default a scope attribute that wasn't supplied, without specifying it

@AckerApple
Copy link

Hey, I get it though, directives being interface like and being an implementation contract of sorts. But I'm also just dealing with this "issue" splitting my dick with every directive I've ever made that defaulted an optional scope attribute. O well

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

No branches or pull requests

4 participants