fix(compiler-cli): perform DOM schema checks even in basic mode in g3 (#38943)

In Ivy, template type-checking has 3 modes: basic, full, and strict. The
primary difference between basic and full modes is that basic mode only
checks the top-level template, whereas full mode descends into nested
templates (embedded views like ngIfs and ngFors). Ivy applies this approach
to all of its template type-checking, including the DOM schema checks which
validate whether an element is a valid component/directive or not.

View Engine has both the basic and the full mode, with the same distinction.
However in View Engine, DOM schema checks happen for the full template even
in the basic mode.

Ivy's behavior here is technically a "fix" as it does not make sense for
some checks to apply to the full template and others only to the top-level
view. However, since g3 relies exclusively on the basic mode of checking and
developers there are used to DOM checks applying throughout their template,
this commit re-enables the nested schema checks even in basic mode only in
g3. This is done by enabling the checks only when Closure Compiler
annotations are requested.

Outside of g3, it's recommended that applications use at least the full mode
of checking (controlled by the `fullTemplateTypeCheck` flag), and ideally
the strict mode (`strictTemplates`).

PR Close #38943
This commit is contained in:
Alex Rickabaugh 2020-09-22 16:18:09 -04:00
parent ffe89fb07d
commit 40975e06c6
6 changed files with 79 additions and 0 deletions

View File

@ -423,6 +423,7 @@ export class NgCompiler {
applyTemplateContextGuards: strictTemplates, applyTemplateContextGuards: strictTemplates,
checkQueries: false, checkQueries: false,
checkTemplateBodies: true, checkTemplateBodies: true,
alwaysCheckSchemaInTemplateBodies: true,
checkTypeOfInputBindings: strictTemplates, checkTypeOfInputBindings: strictTemplates,
honorAccessModifiersForInputBindings: false, honorAccessModifiersForInputBindings: false,
strictNullInputBindings: strictTemplates, strictNullInputBindings: strictTemplates,
@ -451,6 +452,9 @@ export class NgCompiler {
applyTemplateContextGuards: false, applyTemplateContextGuards: false,
checkQueries: false, checkQueries: false,
checkTemplateBodies: false, checkTemplateBodies: false,
// Enable deep schema checking in "basic" template type-checking mode only if Closure
// compilation is requested, which is a good proxy for "only in google3".
alwaysCheckSchemaInTemplateBodies: this.closureCompilerEnabled,
checkTypeOfInputBindings: false, checkTypeOfInputBindings: false,
strictNullInputBindings: false, strictNullInputBindings: false,
honorAccessModifiersForInputBindings: false, honorAccessModifiersForInputBindings: false,

View File

@ -230,6 +230,12 @@ export interface TypeCheckingConfig {
*/ */
checkTemplateBodies: boolean; checkTemplateBodies: boolean;
/**
* Whether to always apply DOM schema checks in template bodies, independently of the
* `checkTemplateBodies` setting.
*/
alwaysCheckSchemaInTemplateBodies: boolean;
/** /**
* Whether to check resolvable queries. * Whether to check resolvable queries.
* *

View File

@ -1298,6 +1298,8 @@ class Scope {
this.templateCtxOpMap.set(node, ctxIndex); this.templateCtxOpMap.set(node, ctxIndex);
if (this.tcb.env.config.checkTemplateBodies) { if (this.tcb.env.config.checkTemplateBodies) {
this.opQueue.push(new TcbTemplateBodyOp(this.tcb, this, node)); this.opQueue.push(new TcbTemplateBodyOp(this.tcb, this, node));
} else if (this.tcb.env.config.alwaysCheckSchemaInTemplateBodies) {
this.appendDeepSchemaChecks(node.children);
} }
this.checkAndAppendReferencesOfNode(node); this.checkAndAppendReferencesOfNode(node);
} else if (node instanceof TmplAstBoundText) { } else if (node instanceof TmplAstBoundText) {
@ -1401,6 +1403,33 @@ class Scope {
this.opQueue.push(new TcbUnclaimedOutputsOp(this.tcb, this, node, claimedOutputs)); this.opQueue.push(new TcbUnclaimedOutputsOp(this.tcb, this, node, claimedOutputs));
} }
} }
private appendDeepSchemaChecks(nodes: TmplAstNode[]): void {
for (const node of nodes) {
if (!(node instanceof TmplAstElement || node instanceof TmplAstTemplate)) {
continue;
}
if (node instanceof TmplAstElement) {
const claimedInputs = new Set<string>();
const directives = this.tcb.boundTarget.getDirectivesOfNode(node);
let hasDirectives: boolean;
if (directives === null || directives.length === 0) {
hasDirectives = false;
} else {
hasDirectives = true;
for (const dir of directives) {
for (const propertyName of dir.inputs.propertyNames) {
claimedInputs.add(propertyName);
}
}
}
this.opQueue.push(new TcbDomSchemaCheckerOp(this.tcb, node, !hasDirectives, claimedInputs));
}
this.appendDeepSchemaChecks(node.children);
}
}
} }
interface TcbBoundInput { interface TcbBoundInput {

View File

@ -167,6 +167,7 @@ export const ALL_ENABLED_CONFIG: TypeCheckingConfig = {
applyTemplateContextGuards: true, applyTemplateContextGuards: true,
checkQueries: false, checkQueries: false,
checkTemplateBodies: true, checkTemplateBodies: true,
alwaysCheckSchemaInTemplateBodies: true,
checkTypeOfInputBindings: true, checkTypeOfInputBindings: true,
honorAccessModifiersForInputBindings: true, honorAccessModifiersForInputBindings: true,
strictNullInputBindings: true, strictNullInputBindings: true,
@ -238,6 +239,7 @@ export function tcb(
checkTypeOfNonDomReferences: true, checkTypeOfNonDomReferences: true,
checkTypeOfPipes: true, checkTypeOfPipes: true,
checkTemplateBodies: true, checkTemplateBodies: true,
alwaysCheckSchemaInTemplateBodies: true,
strictSafeNavigationTypes: true, strictSafeNavigationTypes: true,
useContextGenericType: true, useContextGenericType: true,
strictLiteralTypes: true, strictLiteralTypes: true,

View File

@ -700,6 +700,7 @@ describe('type check blocks', () => {
applyTemplateContextGuards: true, applyTemplateContextGuards: true,
checkQueries: false, checkQueries: false,
checkTemplateBodies: true, checkTemplateBodies: true,
alwaysCheckSchemaInTemplateBodies: true,
checkTypeOfInputBindings: true, checkTypeOfInputBindings: true,
honorAccessModifiersForInputBindings: false, honorAccessModifiersForInputBindings: false,
strictNullInputBindings: true, strictNullInputBindings: true,

View File

@ -390,6 +390,43 @@ runInEachFileSystem(os => {
expect(jsContents).toContain('/** @nocollapse */ TestCmp.ɵcmp'); expect(jsContents).toContain('/** @nocollapse */ TestCmp.ɵcmp');
}); });
it('should still perform schema checks in embedded views', () => {
env.tsconfig({
'fullTemplateTypeCheck': false,
'annotateForClosureCompiler': true,
'ivyTemplateTypeCheck': true,
});
env.write('test.ts', `
import {Component, Directive, NgModule} from '@angular/core';
@Component({
selector: 'test-cmp',
template: \`
<ng-template>
<some-dir>Has a directive, should be okay</some-dir>
<not-a-cmp>Should trigger a schema error</not-a-cmp>
</ng-template>
\`
})
export class TestCmp {}
@Directive({
selector: 'some-dir',
})
export class TestDir {}
@NgModule({
declarations: [TestCmp, TestDir],
})
export class TestModule {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.SCHEMA_INVALID_ELEMENT));
expect(ts.flattenDiagnosticMessageText(diags[0].messageText, '\n'))
.toContain('not-a-cmp');
});
/** /**
* The following set of tests verify that after Tsickle run we do not have cases * The following set of tests verify that after Tsickle run we do not have cases
* which trigger automatic semicolon insertion, which breaks the code. In order * which trigger automatic semicolon insertion, which breaks the code. In order