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

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

Closed
wesleycho wants to merge 1 commit into
angular:masterfrom
wesleycho:feat/error-bad-camelcasing
Closed

feat($compile): Throw error on incorrect casing#11281
wesleycho wants to merge 1 commit into
angular:masterfrom
wesleycho:feat/error-bad-camelcasing

Conversation

@wesleycho

@wesleycho wesleycho commented Mar 9, 2015

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

This addresses #11109 .

lgalfaso
lgalfaso reviewed Mar 9, 2015
View reviewed changes
Comment thread src/ng/compile.js Outdated

@lgalfaso lgalfaso Mar 9, 2015

Copy link
Copy Markdown
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

@wesleycho
wesleycho force-pushed the feat/error-bad-camelcasing branch from 6389355 to 9d842a1 Compare March 11, 2015 03:12
lgalfaso
lgalfaso reviewed Mar 11, 2015
View reviewed changes
Comment thread src/ng/compile.js Outdated

@lgalfaso lgalfaso Mar 11, 2015

Copy link
Copy Markdown
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)) {

@wesleycho
feat($compile): Throw error on incorrect casing
4e2e3aa 
- 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 !
@wesleycho
wesleycho force-pushed the feat/error-bad-camelcasing branch from e8d33dc to 4e2e3aa Compare March 14, 2015 19:06
@petebacondarwin

petebacondarwin commented Mar 17, 2015

Copy link
Copy Markdown
Contributor

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

petebacondarwin commented Mar 17, 2015

Copy link
Copy Markdown
Contributor

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
@wesleycho @petebacondarwin
fix($compile): throw error on invalid directive name
170ff9a 
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
@wesleycho @petebacondarwin
fix($compile): throw error on invalid directive name
634e467 
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
@wesleycho @netman92
fix($compile): throw error on invalid directive name
8be4f95 
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.

Reviewers

No reviews

Assignees

No one assigned

Labels

cla: yes component: $compile needs: work

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@wesleycho @petebacondarwin @lgalfaso @Narretz @rexyolo13 @googlebot