angular / angular.js

AngularJS - HTML enhanced for web apps!
https://angularjs.org
MIT License
58.79k stars 27.48k forks source link

Directive requiring can't find camelCased directive with a data prefix #10722

Open moneytree-doug opened 9 years ago

moneytree-doug commented 9 years ago

This is either a configuration issue or a parsing issue. I keep getting these errors: https://docs.angularjs.org/error/$compile/ctreq?p0=superhero&p1=strength https://docs.angularjs.org/error/$compile/ctreq?p0=super-hero&p1=invis

The code is in the plnkr below. http://plnkr.co/edit/Ofq6OXE1tTTIIKMCZic6

I would expect the consistent camelCasing to work, but that doesnt see to be the case here.

pkozlowski-opensource commented 9 years ago

I'm not sure I understand the problem... In your invis directive you've got require: "^super-hero", while you should have require: "^superHero", - the same way you've got it in the strength directive.

What do you mean exactly by:

I would expect the consistent camelCasing to work

moneytree-doug commented 9 years ago

@pkozlowski-opensource I updated the plunkr, so I found out that the bug only happens when you add data as a prefix to the directive. Basically, if my directive was named dataHero then I would expect the element <data-hero></data-hero> to work, and it does. However, when I require it with ^dataHero then it is unable to find the correct controller. The easier fix here is to just not use data as a prefix, but I thought I would report it anyway.

frfancha commented 9 years ago

@moneytree-doug data is a just decorator which is skipped in directive search, you add data- in html just to pass validations. So, if your directive is named TotoLulu, you can use it with: <toto-lulu> or <data-toto-lulu> and to require it you use its name: require: totoLulu But don't name your directive dataTotoLulu, even if it could work in some use cases, it is highly confusing with the general use of 'data-' in directive name, and avoiding confusion in your code is always a good idea ;-)

moneytree-doug commented 9 years ago

@frfancha I agree completely, but having data prefixed in the element is different from prefixing data to an attribute. But the better error here would be to say not use a data prefix instead of saying the controller cannot be found

Narretz commented 9 years ago

having data prefixed in the element is different from prefixing data to an attribute

I don't see why, actually.

pkozlowski-opensource commented 9 years ago

@moneytree-doug the data prefix has special meaning in AngularJS directives (as described in the "Matching Directives" section of https://docs.angularjs.org/guide/directive) and this prefix is stripped from directive names. So yes, you shouldn't be really naming your directives with the data prefix.

As to:

having data prefixed in the element is different from prefixing data to an attribute

this is debatable - I'm not sure what spec are saying about data prefix on the element names. Would be great if anyone could do a bit of searching / reading on the topic.

I'm going to keep it open for a while as I'm not sure what is the best course of action here: either trying to "fix" this or just update docs....

gkalpak commented 9 years ago

BTW, (not that I recommend this, but) you can still use an element directive starting with data, by prefixing all occurences with data- or x-, e.g.:

<x-data-hero>...</x-data-hero>
or
<data-data-hero>...</data-data-hero>
moneytree-doug commented 9 years ago

I would rather just have Angular restrict a data prefix for a directive name than telling me that it cannot find the correct controller. It just basically diverts my attention away from the correct issue, and I end up spending a few hours figuring out if its configured properly and having to debug Angular itself to figure out the issue.

pkozlowski-opensource commented 9 years ago

@moneytree-doug actually you are onto something - it is true that we could throw when a directive is named starting with one of the "restricted" prefixes.

@petebacondarwin WDYT?

gkalpak commented 9 years ago

...but they are not "restricted" :(

pkozlowski-opensource commented 9 years ago

@gkalpak sure, but as you've said, you wouldn't recommend people to use they directives like <x-x-mydirective>, <data-data-mydirective> or <x-data-mydirective>, right?

Maybe the right thing to do here would be to have this warning in ng-hint?

gkalpak commented 9 years ago

I am just saying that they are not "restricted" directives (right now) as there is a way to use them without problems. So, although I wouldn't be opposed to making them restricted (and throwing an error), it would be a breaking change. (Not that I have anything against breaking changes; just pointing it out :))

In any case, if we decide to "keep" them, raising awareness on the directive normalization process would be nice. There is only a short mention in the $compile docs (L530); and of course in the developer guide, although I am not sure how many people actually read the guide.

:+1: for ngHint wanring !