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

feat($compile): Throw error on incorrect casing #11281

Closed

Conversation

wesleycho
Copy link
Contributor

  • Adds error message for when the first letter of the directive name is
    not capitalized in the definition

This addresses #11109 .

@@ -779,6 +779,14 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
return bindings;
}

function assertFirstLetterIsLowerCased(name) {
var letter = name.charAt(0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use the lowercase function as it handles some locale issues

@@ -779,6 +779,14 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
return bindings;
}

function assertValidDirectiveName(name) {
var letter = name.charAt(0);
if (letter && (letter !== lowercase(letter))) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be if (!letter || letter !== lowercase(letter)) {

- Adds error message for when the first letter of the directive name is
  not capitalized in the definition

- Refactor according to CR

- More fixes as per CR

- Change back to &&

- Change back, add missing !
@petebacondarwin
Copy link
Member

We can't use ng:badname as this refers to using hasOwnProperty where we shouldn't, see https://docs.angularjs.org/error/ng/badname
I'll knock up a new error and merge this. Thanks @wesleycho (and @lgalfaso and @Narretz for reviewing)

@petebacondarwin
Copy link
Member

Moreover, the test was not actually running, since you have to also request that the injector instantiates the $compile service. You could see that the test was not failing by removing the "fix".

It is better to follow the example of the hasOwnProperty test: https://github.com/angular/angular.js/blob/master/test/ng/compileSpec.js#L196-L203

I will fix this up too.

petebacondarwin pushed a commit that referenced this pull request Mar 17, 2015
Directive names must start with a lower case letter.
Previously the compiler would quietly fail.
This change adds an assertion that fails if this is not the case.

Closes #11281
Closes #11109
petebacondarwin pushed a commit that referenced this pull request Mar 17, 2015
Directive names must start with a lower case letter.
Previously the compiler would quietly fail.
This change adds an assertion that fails if this is not the case.

Closes #11281
Closes #11109
netman92 pushed a commit to netman92/angular.js that referenced this pull request Aug 8, 2015
Directive names must start with a lower case letter.
Previously the compiler would quietly fail.
This change adds an assertion that fails if this is not the case.

Closes angular#11281
Closes angular#11109
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.

None yet

6 participants