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

fix($compile): reference correct directive name in ctreq error #7067

Closed
wants to merge 1 commit into from

Conversation

caitp
Copy link
Contributor

@caitp caitp commented Apr 10, 2014

Previously, ctreq would possibly reference the incorrect directive name, due to relying on a directiveName living outside of the closure which throws the exception, which can change before the call is ever made.

This change saves the current value of directiveName as a property of the link function, which prevents this from occurring.

Closes #7062
Closes #7059

@caitp
Copy link
Contributor Author

caitp commented Apr 10, 2014

With a bit more testing, it appears that the directive name matters here.

If the directive requiring "ngModel" is called "zDir", and the only other directive used is "ngClick", then the correct error message is used, because "zDir" is the last directive referenced.

Given that, I'm going to re-word my test case a bit so that it's clear what is actually being tested.

Previously, ctreq would possibly reference the incorrect directive name,
due to relying on a directiveName living outside of the closure which
throws the exception, which can change before the call is ever made.

This change saves the current value of directiveName as a property of
the link function, which prevents this from occurring.

Closes angular#7062
@IgorMinar
Copy link
Contributor

lgtm

caitp added a commit that referenced this pull request Apr 15, 2014
Previously, ctreq would possibly reference the incorrect directive name,
due to relying on a directiveName living outside of the closure which
throws the exception, which can change before the call is ever made.

This change saves the current value of directiveName as a property of
the link function, which prevents this from occurring.

Closes #7062
Closes #7067
@caitp caitp closed this in 1192531 Apr 15, 2014
@devamir
Copy link

devamir commented Jun 8, 2014

Hey ,

I am new to angular and facing the same issue but don't know which files to include from your fixes.
Currently i have only one angular.js which i am using but here i can see lots of files.

@caitp
Copy link
Contributor Author

caitp commented Jun 8, 2014

@devamir it's in https://code.angularjs.org/1.3.0-beta.6/angular.js and https://code.angularjs.org/1.2.17/angular.js --- but it may not be a fix for the specific issue you're having, the compiler still has some errors that behave weirdly

@devamir
Copy link

devamir commented Jun 8, 2014

what is the difference between the conventional angularjs.js which we use and this https://code.angularjs.org/1.2.17/angular.js i think they are one and the same and regarding my issue
i have used the ui.bootstrap carousel and only IE8 is not supporting and producing issue that
"Controller 'carousel', required by directive 'ngTransclude', can't be found!"

@caitp
Copy link
Contributor Author

caitp commented Jun 8, 2014

What I'm saying is that this patch is included in releases 1.2.17 (which was just released the other day) and 1.3.0-beta.6 (released a while ago)

@devamir
Copy link

devamir commented Jun 8, 2014

ohhk got it Thnx @caitp

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