Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Schematics class name validation #23485

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

alan-agius4
Copy link
Collaborator

@alan-agius4 alan-agius4 commented Jul 1, 2022

fix(@angular-devkit/core): classify string util should concat string without using a .

. is not a valid character in ES6 class names.

Prior to this change foo.module before used to be incorrectly classified to Foo.Module instead of FooModule.

Closes #13824


fix(@schematics/angular): prevent numbers from class names

With this change we prevent creating classes with invalid characters.

Closes #12868


Combined into a single PR as they are dependent on each other.

…g without using a `.`

`.` is not a valid character in ES6 class names.

Prior to this change `foo.module` before used to be incorrectly classified to `Foo.Module` instead of `FooModule`.

Closes angular#13824
@alan-agius4 alan-agius4 added action: review target: patch 2022Q3 Fixit labels Jul 1, 2022
@alan-agius4 alan-agius4 force-pushed the class-name-validation branch 2 times, most recently from d271e42 to 8d8faec Compare Jul 1, 2022
@alan-agius4 alan-agius4 changed the title Class name validation Schematics class name validation Jul 1, 2022
@alan-agius4 alan-agius4 requested a review from clydin Jul 1, 2022
packages/schematics/angular/utility/validation.ts Outdated Show resolved Hide resolved
@@ -89,7 +90,7 @@ export function classify(str: string): string {
return str
.split('.')
.map((part) => capitalize(camelize(part)))
.join('.');
.join('');
Copy link
Member

@clydin clydin Jul 1, 2022

Choose a reason for hiding this comment

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

I'm wondering if this would be a breaking change since it is public and it could be expected to operate in the previous manner.

Copy link
Collaborator Author

@alan-agius4 alan-agius4 Jul 1, 2022

Choose a reason for hiding this comment

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

Umm… not sure show to classify this since technically it is was an incorrect behaviour.

Maybe we should discuss it during our next sync.

With this change we prevent creating classes with invalid characters.

Closes angular#12868
@alan-agius4 alan-agius4 added the need: further discussion label Jul 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2022Q3 Fixit action: review need: further discussion target: patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants