Skip to content
Permalink
Browse files

feat(ivy): use the schema registry to check DOM bindings (#32171)

Previously, ngtsc attempted to use the .d.ts schema for HTML elements to
check bindings to DOM properties. However, the TypeScript lib.dom.d.ts
schema does not perfectly align with the Angular DomElementSchemaRegistry,
and these inconsistencies would cause issues in apps. There is also the
concern of supporting both CUSTOM_ELEMENTS_SCHEMA and NO_ERRORS_SCHEMA which
would have been very difficult to do in the existing system.

With this commit, the DomElementSchemaRegistry is employed in ngtsc to check
bindings to the DOM. Previous work on producing template diagnostics is used
to support generation of this different kind of error with the same high
quality of error message.

PR Close #32171
  • Loading branch information...
alxhub authored and atscott committed Aug 16, 2019
1 parent 904a201 commit 0677cf0cbebca42e0441b9596c0fc4faa6da44b8
Showing with 574 additions and 122 deletions.
  1. +5 −2 packages/compiler-cli/src/ngtsc/annotations/src/component.ts
  2. +42 −2 packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts
  3. +10 −0 packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts
  4. +2 −1 packages/compiler-cli/src/ngtsc/metadata/src/api.ts
  5. +1 −0 packages/compiler-cli/src/ngtsc/metadata/src/dts.ts
  6. +5 −2 packages/compiler-cli/src/ngtsc/program.ts
  7. +3 −1 packages/compiler-cli/src/ngtsc/scope/src/local.ts
  8. +16 −2 packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts
  9. +1 −0 packages/compiler-cli/src/ngtsc/typecheck/BUILD.bazel
  10. +23 −3 packages/compiler-cli/src/ngtsc/typecheck/src/api.ts
  11. +20 −68 packages/compiler-cli/src/ngtsc/typecheck/src/context.ts
  12. +16 −5 packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts
  13. +92 −0 packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts
  14. +78 −0 packages/compiler-cli/src/ngtsc/typecheck/src/source.ts
  15. +66 −8 packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
  16. +5 −2 packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts
  17. +2 −2 packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts
  18. +21 −6 packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts
  19. +39 −18 packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts
  20. +3 −0 packages/compiler-cli/test/ngtsc/fake_core/index.ts
  21. +123 −0 packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
  22. +1 −0 packages/compiler/src/compiler.ts
@@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {ConstantPool, CssSelector, DEFAULT_INTERPOLATION_CONFIG, DomElementSchemaRegistry, Expression, ExternalExpr, InterpolationConfig, LexerRange, ParseError, ParseSourceFile, ParseTemplateOptions, R3ComponentMetadata, R3TargetBinder, SelectorMatcher, Statement, TmplAstNode, WrappedNodeExpr, compileComponentFromMetadata, makeBindingParser, parseTemplate} from '@angular/compiler';
import {ConstantPool, CssSelector, DEFAULT_INTERPOLATION_CONFIG, DomElementSchemaRegistry, Expression, ExternalExpr, InterpolationConfig, LexerRange, ParseError, ParseSourceFile, ParseTemplateOptions, R3ComponentMetadata, R3TargetBinder, SchemaMetadata, SelectorMatcher, Statement, TmplAstNode, WrappedNodeExpr, compileComponentFromMetadata, makeBindingParser, parseTemplate} from '@angular/compiler';
import * as ts from 'typescript';

import {CycleAnalyzer} from '../../cycles';
@@ -396,6 +396,7 @@ export class ComponentDecoratorHandler implements

const matcher = new SelectorMatcher<DirectiveMeta>();
const pipes = new Map<string, Reference<ClassDeclaration<ts.ClassDeclaration>>>();
let schemas: SchemaMetadata[] = [];

const scope = this.scopeReader.getScopeForComponent(node);
if (scope !== null) {
@@ -410,10 +411,12 @@ export class ComponentDecoratorHandler implements
}
pipes.set(name, ref as Reference<ClassDeclaration<ts.ClassDeclaration>>);
}
schemas = scope.schemas;
}

const bound = new R3TargetBinder(matcher).bind({template: template.nodes});
ctx.addTemplate(new Reference(node), bound, pipes, meta.templateSourceMapping, template.file);
ctx.addTemplate(
new Reference(node), bound, pipes, schemas, meta.templateSourceMapping, template.file);
}

resolve(node: ClassDeclaration, analysis: ComponentHandlerData): ResolveResult {
@@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Expression, ExternalExpr, InvokeFunctionExpr, LiteralArrayExpr, LiteralExpr, R3Identifiers, R3InjectorMetadata, R3NgModuleMetadata, R3Reference, STRING_TYPE, Statement, WrappedNodeExpr, compileInjector, compileNgModule} from '@angular/compiler';
import {CUSTOM_ELEMENTS_SCHEMA, Expression, ExternalExpr, InvokeFunctionExpr, LiteralArrayExpr, LiteralExpr, NO_ERRORS_SCHEMA, R3Identifiers, R3InjectorMetadata, R3NgModuleMetadata, R3Reference, STRING_TYPE, SchemaMetadata, Statement, WrappedNodeExpr, compileInjector, compileNgModule} from '@angular/compiler';
import * as ts from 'typescript';

import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
@@ -122,6 +122,45 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
bootstrapRefs = this.resolveTypeList(expr, bootstrapMeta, name, 'bootstrap');
}

const schemas: SchemaMetadata[] = [];
if (ngModule.has('schemas')) {
const rawExpr = ngModule.get('schemas') !;
const result = this.evaluator.evaluate(rawExpr);
if (!Array.isArray(result)) {
throw new FatalDiagnosticError(
ErrorCode.VALUE_HAS_WRONG_TYPE, rawExpr, `NgModule.schemas must be an array`);
}

for (const schemaRef of result) {
if (!(schemaRef instanceof Reference)) {
throw new FatalDiagnosticError(
ErrorCode.VALUE_HAS_WRONG_TYPE, rawExpr,
'NgModule.schemas must be an array of schemas');
}
const id = schemaRef.getIdentityIn(schemaRef.node.getSourceFile());
if (id === null || schemaRef.ownedByModuleGuess !== '@angular/core') {
throw new FatalDiagnosticError(
ErrorCode.VALUE_HAS_WRONG_TYPE, rawExpr,
'NgModule.schemas must be an array of schemas');
}
// Since `id` is the `ts.Identifer` within the schema ref's declaration file, it's safe to
// use `id.text` here to figure out which schema is in use. Even if the actual reference was
// renamed when the user imported it, these names will match.
switch (id.text) {
case 'CUSTOM_ELEMENTS_SCHEMA':
schemas.push(CUSTOM_ELEMENTS_SCHEMA);
break;
case 'NO_ERRORS_SCHEMA':
schemas.push(NO_ERRORS_SCHEMA);
break;
default:
throw new FatalDiagnosticError(
ErrorCode.VALUE_HAS_WRONG_TYPE, rawExpr,
`'${schemaRef.debugName}' is not a valid NgModule schema`);
}
}
}

const id: Expression|null =
ngModule.has('id') ? new WrappedNodeExpr(ngModule.get('id') !) : null;

@@ -130,9 +169,10 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
// computation.
this.metaRegistry.registerNgModuleMetadata({
ref: new Reference(node),
schemas,
declarations: declarationRefs,
imports: importRefs,
exports: exportRefs
exports: exportRefs,
});

const valueContext = node.getSourceFile();
@@ -73,6 +73,16 @@ export enum ErrorCode {
* expression.
*/
NGCC_MIGRATION_DYNAMIC_BASE_CLASS = 7003,

/**
* An element name failed validation against the DOM schema.
*/
SCHEMA_INVALID_ELEMENT = 8001,

/**
* An element's attribute name failed validation against the DOM schema.
*/
SCHEMA_INVALID_ATTRIBUTE = 8002,
}

export function ngErrorCode(code: ErrorCode): number {
@@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {DirectiveMeta as T2DirectiveMeta} from '@angular/compiler';
import {DirectiveMeta as T2DirectiveMeta, SchemaMetadata} from '@angular/compiler';

import {Reference} from '../../imports';
import {ClassDeclaration} from '../../reflection';
@@ -19,6 +19,7 @@ export interface NgModuleMeta {
declarations: Reference<ClassDeclaration>[];
imports: Reference<ClassDeclaration>[];
exports: Reference<ClassDeclaration>[];
schemas: SchemaMetadata[];
}

/**
@@ -59,6 +59,7 @@ export class DtsMetadataReader implements MetadataReader {
this.checker, exportMetadata, ref.ownedByModuleGuess, resolutionContext),
imports: extractReferencesFromType(
this.checker, importMetadata, ref.ownedByModuleGuess, resolutionContext),
schemas: [],
};
}

@@ -400,7 +400,9 @@ export class NgtscProgram implements api.Program {
applyTemplateContextGuards: true,
checkQueries: false,
checkTemplateBodies: true,
checkTypeOfBindings: true,
checkTypeOfInputBindings: true,
// Even in full template type-checking mode, DOM binding checks are not quite ready yet.
checkTypeOfDomBindings: false,
checkTypeOfPipes: true,
strictSafeNavigationTypes: true,
};
@@ -409,7 +411,8 @@ export class NgtscProgram implements api.Program {
applyTemplateContextGuards: false,
checkQueries: false,
checkTemplateBodies: false,
checkTypeOfBindings: false,
checkTypeOfInputBindings: false,
checkTypeOfDomBindings: false,
checkTypeOfPipes: false,
strictSafeNavigationTypes: false,
};
@@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {ExternalExpr} from '@angular/compiler';
import {ExternalExpr, SchemaMetadata} from '@angular/compiler';
import * as ts from 'typescript';

import {ErrorCode, makeDiagnostic} from '../../diagnostics';
@@ -28,6 +28,7 @@ export interface LocalNgModuleData {
export interface LocalModuleScope extends ExportScope {
compilation: ScopeData;
reexports: Reexport[]|null;
schemas: SchemaMetadata[];
}

/**
@@ -375,6 +376,7 @@ export class LocalModuleScopeRegistry implements MetadataRegistry, ComponentScop
},
exported,
reexports,
schemas: ngModule.schemas,
};
this.cache.set(ref.node, scope);
return scope;
@@ -52,6 +52,7 @@ describe('LocalModuleScopeRegistry', () => {
imports: [],
declarations: [Dir1, Dir2, Pipe1],
exports: [Dir1, Pipe1],
schemas: [],
});

const scope = scopeRegistry.getScopeOfModule(Module.node) !;
@@ -67,18 +68,21 @@ describe('LocalModuleScopeRegistry', () => {
imports: [ModuleB],
declarations: [DirA],
exports: [],
schemas: [],
});
metaRegistry.registerNgModuleMetadata({
ref: new Reference(ModuleB.node),
exports: [ModuleC, DirB],
declarations: [DirB],
imports: []
imports: [],
schemas: [],
});
metaRegistry.registerNgModuleMetadata({
ref: new Reference(ModuleC.node),
declarations: [DirCI, DirCE],
exports: [DirCE],
imports: []
imports: [],
schemas: [],
});

const scopeA = scopeRegistry.getScopeOfModule(ModuleA.node) !;
@@ -94,12 +98,14 @@ describe('LocalModuleScopeRegistry', () => {
exports: [ModuleB],
imports: [],
declarations: [],
schemas: [],
});
metaRegistry.registerNgModuleMetadata({
ref: new Reference(ModuleB.node),
declarations: [Dir],
exports: [Dir],
imports: [],
schemas: [],
});

const scopeA = scopeRegistry.getScopeOfModule(ModuleA.node) !;
@@ -115,18 +121,21 @@ describe('LocalModuleScopeRegistry', () => {
declarations: [DirA, DirA],
imports: [ModuleB, ModuleC],
exports: [DirA, DirA, DirB, ModuleB],
schemas: [],
});
metaRegistry.registerNgModuleMetadata({
ref: new Reference(ModuleB.node),
declarations: [DirB],
imports: [],
exports: [DirB],
schemas: [],
});
metaRegistry.registerNgModuleMetadata({
ref: new Reference(ModuleC.node),
declarations: [],
imports: [],
exports: [ModuleB],
schemas: [],
});

const scope = scopeRegistry.getScopeOfModule(ModuleA.node) !;
@@ -149,6 +158,7 @@ describe('LocalModuleScopeRegistry', () => {
exports: [],
imports: [],
declarations: [DirInModule],
schemas: [],
});

const scope = scopeRegistry.getScopeOfModule(Module.node) !;
@@ -163,12 +173,14 @@ describe('LocalModuleScopeRegistry', () => {
exports: [Dir],
imports: [ModuleB],
declarations: [],
schemas: [],
});
metaRegistry.registerNgModuleMetadata({
ref: new Reference(ModuleB.node),
declarations: [Dir],
exports: [Dir],
imports: [],
schemas: [],
});

const scopeA = scopeRegistry.getScopeOfModule(ModuleA.node) !;
@@ -183,12 +195,14 @@ describe('LocalModuleScopeRegistry', () => {
exports: [Dir],
imports: [],
declarations: [],
schemas: [],
});
metaRegistry.registerNgModuleMetadata({
ref: new Reference(ModuleB.node),
declarations: [Dir],
exports: [Dir],
imports: [],
schemas: [],
});

expect(scopeRegistry.getScopeOfModule(ModuleA.node)).toBe(null);
@@ -8,6 +8,7 @@ ts_library(
deps = [
"//packages:types",
"//packages/compiler",
"//packages/compiler-cli/src/ngtsc/diagnostics",
"//packages/compiler-cli/src/ngtsc/file_system",
"//packages/compiler-cli/src/ngtsc/imports",
"//packages/compiler-cli/src/ngtsc/metadata",
@@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {BoundTarget, DirectiveMeta} from '@angular/compiler';
import {BoundTarget, DirectiveMeta, SchemaMetadata} from '@angular/compiler';
import * as ts from 'typescript';

import {Reference} from '../../imports';
@@ -45,6 +45,11 @@ export interface TypeCheckBlockMetadata {
* Pipes used in the template of the component.
*/
pipes: Map<string, Reference<ClassDeclaration<ts.ClassDeclaration>>>;

/**
* Schemas that apply to this template.
*/
schemas: SchemaMetadata[];
}

export interface TypeCtorMetadata {
@@ -72,8 +77,23 @@ export interface TypeCheckingConfig {
* checked, but not the assignment of the resulting type to the `input` property of whichever
* directive or component is receiving the binding. If set to `true`, both sides of the assignment
* are checked.
*
* This flag only affects bindings to components/directives. Bindings to the DOM are checked if
* `checkTypeOfDomBindings` is set.
*/
checkTypeOfInputBindings: boolean;

/**
* Whether to check the left-hand side type of binding operations to DOM properties.
*
* As `checkTypeOfBindings`, but only applies to bindings to DOM properties.
*
* This does not affect the use of the `DomSchemaChecker` to validate the template against the DOM
* schema. Rather, this flag is an experimental, not yet complete feature which uses the
* lib.dom.d.ts DOM typings in TypeScript to validate that DOM bindings are of the correct type
* for assignability to the underlying DOM element properties.
*/
checkTypeOfBindings: boolean;
checkTypeOfDomBindings: boolean;

/**
* Whether to include type information from pipes in the type-checking operation.
@@ -154,4 +174,4 @@ export interface ExternalTemplateSourceMapping {
node: ts.Expression;
template: string;
templateUrl: string;
}
}

0 comments on commit 0677cf0

Please sign in to comment.
You can’t perform that action at this time.