refactor(schematics): enable strict null checks #12983
Conversation
devversion
added
pr: merge safe
target: major
googlebot
added
the
cla: yes
| @@ -30,7 +30,7 @@ export default function(options: Schema): Rule { | |||
| */ | |||
| function addFormModulesToModule(options: Schema) { | |||
| return (host: Tree) => { | |||
| const modulePath = findModuleFromOptions(host, options); | |||
| const modulePath = findModuleFromOptions(host, options)!; | |||
There was a problem hiding this comment.
This is never undefined or null because the addXModulesToModule methods are only called if skipImport is set to false. And the findModuleFromOptions method only returns undefined if that option is enabled.
@jelbourn Want me to add a comment for this? (afraid of repeating it for every schematic though)
There was a problem hiding this comment.
Could the function return an empty string in that case?
There was a problem hiding this comment.
What would be the benefit? This comes from an utility function from @schematics/angular. It should be really never undefined because we already check for skipImport before.
| @@ -30,7 +30,7 @@ export default function(options: Schema): Rule { | |||
| */ | |||
| function addFormModulesToModule(options: Schema) { | |||
| return (host: Tree) => { | |||
| const modulePath = findModuleFromOptions(host, options); | |||
| const modulePath = findModuleFromOptions(host, options)!; | |||
There was a problem hiding this comment.
Could the function return an empty string in that case?
| @@ -33,7 +33,7 @@ export function appendElementToHead(host: Tree, project: WorkspaceProject, eleme | |||
| throw `Could not find '<head>' element in HTML file: ${indexPath}`; | |||
| } | |||
|
|
|||
| const endTagOffset = headTag.sourceCodeLocation.endTag.startOffset; | |||
| const endTagOffset = headTag.sourceCodeLocation!.endTag.startOffset; | |||
There was a problem hiding this comment.
Why is this never null?
Similar for other one-off places (like architect below)
There was a problem hiding this comment.
This is never null because we explicitly enabled the parse5 option to track the source code location. So we will always have this property set.
Regarding architect: We could do better here, but I just went with the assumption that every CLI project has an architect block configured. We also assert if the default architect builders are used in the theming/index.ts file.
There was a problem hiding this comment.
We should capture that explanation somewhere in the code
There was a problem hiding this comment.
I will add a note for the parse5 instances. There aren't too much of those.
For the architect. I'll probably create an utility function that does the checking for an architect target. That way we are strict-null check compliant and also make sure that projects that don't have a builder will gracefully exit with a meaningful error message.
* Enables strict null checks for the schematics and fixes a few potential unsafe accesses. * Supports specifying a project for `ng-add`.
devversion
force-pushed
the
refactor/schematics-strict-null-checks
branch
2 times, most recently
from
e7f5c42 to
0a872c4
Compare
devversion
force-pushed
the
refactor/schematics-strict-null-checks
branch
from
0a872c4 to
85ded36
Compare
|
@jelbourn Addressed your feedback. Please have another look |
|
Just looks like there's a lint error |
|
Fixed |
devversion
added
pr: lgtm
action: merge
|
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. |
ng-add.