-
Notifications
You must be signed in to change notification settings - Fork 27.4k
fix(ngTranslude): detect ngTranslude usage without a translusion directive #4162
Conversation
Thanks for the PR!
If you need to make changes to your pull request, you can update the commit with Thanks again for your help! |
@@ -53,6 +53,10 @@ | |||
*/ | |||
var ngTranscludeDirective = ngDirective({ | |||
controller: ['$transclude', function($transclude) { | |||
if (!$transclude) { | |||
throw new Error('Cannot use ng-translude without a translusion directive'); |
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.
this should be a minErr
also it should be marked as "fix" not "chore". (chores don't show up in release notes) |
@@ -1,5 +1,7 @@ | |||
'use strict'; | |||
|
|||
var $ngTranscludeMinErr = minErr('ngTransclude'); |
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.
since you need only one error with this namespace, the convention is to inline the minErr call
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.
That does not seem to be a good convention IMO but sure.
otherwise this looks good to me |
looks good to me |
Actually it needs a spot in the message to interpolate the starting element. It also needs an error page for the docs. I'll fix it. |
Landed as 5a1a6b8 ultimately. |
Closes #3759