fix(@schematics/angular): Allow empty string in the type option #17040
Conversation
sacgrover
force-pushed
the
empty-type
branch
from
d66a704 to
e5297d7
Compare
|
@alan-agius4 / @clydin |
| @@ -22,6 +22,7 @@ export * from './rules/random'; | |||
| export * from './rules/schematic'; | |||
| export * from './rules/template'; | |||
| export * from './rules/url'; | |||
| export * from './rules/rename'; | |||
There was a problem hiding this comment.
This change adds an element to the public API surface. This should be avoided if possible since each addition adds to the long term sustainment cost.
This is also why the bazel build CI is failing. It has a public API check. New public API baselines need to be accepted before the check passes. This helps ensure that new additions were not accidental.
There was a problem hiding this comment.
Instead of rename now I used forEach that was already available in public api
| @@ -49,15 +50,15 @@ function addDeclarationToNgModule(options: ComponentOptions): Rule { | |||
| return host; | |||
| } | |||
|
|
|||
| options.type = !!options.type ? options.type : 'Component'; | |||
| options.type = options.type != null ? options.type : 'Component'; | |||
|
|
|||
| const modulePath = options.module; | |||
| const source = readIntoSourceFile(host, modulePath); | |||
|
|
|||
| const componentPath = `/${options.path}/` | |||
There was a problem hiding this comment.
Since this line is already calculating the file name base, I think we should consider splitting this into two pieces and using them on the templates. “componentPath” can be the first two parts of the current. And “componentFile” can be the remainder as the filename part. This second part can then be used has the base for all file elements in the templates.
This would remove the need to fix the broken file names with a rename as well as optimize the name creation so that it doesn’t have to happen in multiple places as it does currently.
There was a problem hiding this comment.
This componentPath we are using to add component in appModule. We are not using it for template currently.
sacgrover
force-pushed
the
empty-type
branch
from
e5297d7 to
a42ca99
Compare
Currently, Component and Class have the options to add custom type. In the case of class, It's already working fine with an empty string in type but in the case of component When setting the type to an empty string the file names generated will contain an extra period (.) which breaks the flow. With this PR, It will generate the files without an extra period (.) Reference angular#16811 and angular#16891
sacgrover
force-pushed
the
empty-type
branch
from
a42ca99 to
837b797
Compare
|
Commit message changed. |
clydin
changed the title
|
Also merged to 9.0.x patch branch. Fixed a minor merge conflict with the |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Currently, Component and Class have the options to add custom type. In the case of class, It's already working fine with an empty string in type but in the case of component When setting the type to an empty string the file names generated will contain an extra period (.) which breaks the flow.
With this PR, It will generate the files without an extra period (.)
Reference #16811 and #16891