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

feat($compile): throw error when directive name or factory fn is invalid #15057

Merged
merged 1 commit into from
Aug 28, 2016

Conversation

Narretz
Copy link
Contributor

@Narretz Narretz commented Aug 24, 2016

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
feat

What is the current behavior? (You can also link to an open issue here)
when the directive name or factory fn is undefined, no error is thrown

What is the new behavior (if this is a feature change)?
better errors are thrown

Does this PR introduce a breaking change?
No. Although I'm not sure if isFunction will catch all exotic ways to define a function.

Please check if the PR fulfills these requirements

Other information:

Closes: #15056

@gkalpak
Copy link
Member

gkalpak commented Aug 25, 2016

If we do this for directives, why not do the same for controllers, filters, services etc? I feel we are going to far trying to be helpful here.

@Narretz
Copy link
Contributor Author

Narretz commented Aug 25, 2016

Is there a thing as too helpful? :D
The difference between directives and controllers, filters, services is that if the latter haven't been registered, they will complain when they are actually used. Directives are just attributes in DOM elements, so if there's no match than just nothing happens.
I think it's a change we can make - it's very little code.

@gkalpak
Copy link
Member

gkalpak commented Aug 26, 2016

Is there a thing as too helpful? :D

Absolutely 😛

The difference between directives and controllers, filters, services is that if the latter haven't been registered, they will complain when they are actually used.

Point taken 😄
I don't find this necessary, but since it's such a small change I am OK merging this (as long as this not a considered a breaking change).

@Narretz
Copy link
Contributor Author

Narretz commented Aug 26, 2016

I don't think it's a breaking change, as I can't see a use case for adding an "empty" directive. And functions should be detected regardless of which shape they are. I guess I was a bit worried because we had that issue with the constructor fns in compile, but this is something different.

@Narretz Narretz force-pushed the fix-compile-directive-errors branch from d35cbad to a73d514 Compare August 28, 2016 14:11
@Narretz
Copy link
Contributor Author

Narretz commented Aug 28, 2016

I've relaxed the check for the factory, as we can't guarantee 100% that our isFunction detects the most exotic ways of defining a function.

@Narretz Narretz merged commit 53a3bf6 into angular:master Aug 28, 2016
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
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.

Syntax error in directive does not throw exception
3 participants