fix(ivy): split checkTypeOfReferences into DOM and non-DOM flags. (#33365)

View Engine correctly infers the type of local refs to directives or to
<ng-template>s, just not to DOM nodes. This commit splits the
checkTypeOfReferences flag into two separate halves, allowing the compiler
to align with this behavior.

PR Close #33365
This commit is contained in:
Alex Rickabaugh 2019-10-23 17:47:47 -07:00 committed by Andrew Kushnir
parent d8ce2129d5
commit 113411c9b0
5 changed files with 73 additions and 16 deletions

View File

@ -441,7 +441,8 @@ export class NgtscProgram implements api.Program {
// - error TS2531: Object is possibly 'null'. // - error TS2531: Object is possibly 'null'.
// - error TS2339: Property 'value' does not exist on type 'EventTarget'. // - error TS2339: Property 'value' does not exist on type 'EventTarget'.
checkTypeOfDomEvents: false, checkTypeOfDomEvents: false,
checkTypeOfReferences: true, checkTypeOfDomReferences: true,
checkTypeOfNonDomReferences: true,
checkTypeOfPipes: true, checkTypeOfPipes: true,
strictSafeNavigationTypes: true, strictSafeNavigationTypes: true,
}; };
@ -457,7 +458,8 @@ export class NgtscProgram implements api.Program {
checkTypeOfOutputEvents: false, checkTypeOfOutputEvents: false,
checkTypeOfAnimationEvents: false, checkTypeOfAnimationEvents: false,
checkTypeOfDomEvents: false, checkTypeOfDomEvents: false,
checkTypeOfReferences: false, checkTypeOfDomReferences: false,
checkTypeOfNonDomReferences: false,
checkTypeOfPipes: false, checkTypeOfPipes: false,
strictSafeNavigationTypes: false, strictSafeNavigationTypes: false,
}; };

View File

@ -154,14 +154,24 @@ export interface TypeCheckingConfig {
*/ */
checkTypeOfDomEvents: boolean; checkTypeOfDomEvents: boolean;
/**
* Whether to infer the type of local references to DOM elements.
*
* If this is `true`, the type of a `#ref` variable on a DOM node in the template will be
* determined by the type of `document.createElement` for the given DOM node type. If set to
* `false`, the type of `ref` for DOM nodes will be `any`.
*/
checkTypeOfDomReferences: boolean;
/** /**
* Whether to infer the type of local references. * Whether to infer the type of local references.
* *
* If this is `true`, the type of any `#ref` variable in the template will be determined by the * If this is `true`, the type of a `#ref` variable that points to a directive or `TemplateRef` in
* referenced entity (either a directive or a DOM element). If set to `false`, the type of `ref` * the template will be inferred correctly. If set to `false`, the type of `ref` for will be
* will be `any`. * `any`.
*/ */
checkTypeOfReferences: boolean; checkTypeOfNonDomReferences: boolean;
/** /**
* Whether to include type information from pipes in the type-checking operation. * Whether to include type information from pipes in the type-checking operation.

View File

@ -1023,11 +1023,6 @@ class TcbExpressionTranslator {
addParseSpanInfo(expr, toAbsoluteSpan(ast.span, this.sourceSpan)); addParseSpanInfo(expr, toAbsoluteSpan(ast.span, this.sourceSpan));
return expr; return expr;
} else if (binding instanceof TmplAstReference) { } else if (binding instanceof TmplAstReference) {
if (!this.tcb.env.config.checkTypeOfReferences) {
// References are pinned to 'any'.
return NULL_AS_ANY;
}
const target = this.tcb.boundTarget.getReferenceTarget(binding); const target = this.tcb.boundTarget.getReferenceTarget(binding);
if (target === null) { if (target === null) {
throw new Error(`Unbound reference? ${binding.name}`); throw new Error(`Unbound reference? ${binding.name}`);
@ -1037,10 +1032,20 @@ class TcbExpressionTranslator {
// element or template. // element or template.
if (target instanceof TmplAstElement) { if (target instanceof TmplAstElement) {
if (!this.tcb.env.config.checkTypeOfDomReferences) {
// References to DOM nodes are pinned to 'any'.
return NULL_AS_ANY;
}
const expr = ts.getMutableClone(this.scope.resolve(target)); const expr = ts.getMutableClone(this.scope.resolve(target));
addParseSpanInfo(expr, toAbsoluteSpan(ast.span, this.sourceSpan)); addParseSpanInfo(expr, toAbsoluteSpan(ast.span, this.sourceSpan));
return expr; return expr;
} else if (target instanceof TmplAstTemplate) { } else if (target instanceof TmplAstTemplate) {
if (!this.tcb.env.config.checkTypeOfNonDomReferences) {
// References to `TemplateRef`s pinned to 'any'.
return NULL_AS_ANY;
}
// Direct references to an <ng-template> node simply require a value of type // Direct references to an <ng-template> node simply require a value of type
// `TemplateRef<any>`. To get this, an expression of the form // `TemplateRef<any>`. To get this, an expression of the form
// `(null as any as TemplateRef<any>)` is constructed. // `(null as any as TemplateRef<any>)` is constructed.
@ -1053,6 +1058,11 @@ class TcbExpressionTranslator {
addParseSpanInfo(value, toAbsoluteSpan(ast.span, this.sourceSpan)); addParseSpanInfo(value, toAbsoluteSpan(ast.span, this.sourceSpan));
return value; return value;
} else { } else {
if (!this.tcb.env.config.checkTypeOfNonDomReferences) {
// References to directives are pinned to 'any'.
return NULL_AS_ANY;
}
const expr = ts.getMutableClone(this.scope.resolve(target.node, target.directive)); const expr = ts.getMutableClone(this.scope.resolve(target.node, target.directive));
addParseSpanInfo(expr, toAbsoluteSpan(ast.span, this.sourceSpan)); addParseSpanInfo(expr, toAbsoluteSpan(ast.span, this.sourceSpan));
return expr; return expr;

View File

@ -156,7 +156,8 @@ export const ALL_ENABLED_CONFIG: TypeCheckingConfig = {
checkTypeOfOutputEvents: true, checkTypeOfOutputEvents: true,
checkTypeOfAnimationEvents: true, checkTypeOfAnimationEvents: true,
checkTypeOfDomEvents: true, checkTypeOfDomEvents: true,
checkTypeOfReferences: true, checkTypeOfDomReferences: true,
checkTypeOfNonDomReferences: true,
checkTypeOfPipes: true, checkTypeOfPipes: true,
strictSafeNavigationTypes: true, strictSafeNavigationTypes: true,
}; };
@ -207,7 +208,8 @@ export function tcb(
checkTypeOfOutputEvents: true, checkTypeOfOutputEvents: true,
checkTypeOfAnimationEvents: true, checkTypeOfAnimationEvents: true,
checkTypeOfDomEvents: true, checkTypeOfDomEvents: true,
checkTypeOfReferences: true, checkTypeOfDomReferences: true,
checkTypeOfNonDomReferences: true,
checkTypeOfPipes: true, checkTypeOfPipes: true,
checkTemplateBodies: true, checkTemplateBodies: true,
strictSafeNavigationTypes: true, strictSafeNavigationTypes: true,

View File

@ -298,7 +298,8 @@ describe('type check blocks', () => {
checkTypeOfOutputEvents: true, checkTypeOfOutputEvents: true,
checkTypeOfAnimationEvents: true, checkTypeOfAnimationEvents: true,
checkTypeOfDomEvents: true, checkTypeOfDomEvents: true,
checkTypeOfReferences: true, checkTypeOfDomReferences: true,
checkTypeOfNonDomReferences: true,
checkTypeOfPipes: true, checkTypeOfPipes: true,
strictSafeNavigationTypes: true, strictSafeNavigationTypes: true,
}; };
@ -424,7 +425,7 @@ describe('type check blocks', () => {
}); });
}); });
describe('config.checkTypeOfReferences', () => { describe('config.checkTypeOfDomReferences', () => {
const TEMPLATE = `<input #ref>{{ref.value}}`; const TEMPLATE = `<input #ref>{{ref.value}}`;
it('should trace references when enabled', () => { it('should trace references when enabled', () => {
@ -433,12 +434,44 @@ describe('type check blocks', () => {
}); });
it('should use any for reference types when disabled', () => { it('should use any for reference types when disabled', () => {
const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfReferences: false}; const DISABLED_CONFIG:
TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfDomReferences: false};
const block = tcb(TEMPLATE, [], DISABLED_CONFIG); const block = tcb(TEMPLATE, [], DISABLED_CONFIG);
expect(block).toContain('(null as any).value'); expect(block).toContain('(null as any).value');
}); });
}); });
describe('config.checkTypeOfNonDomReferences', () => {
const DIRECTIVES: TestDeclaration[] = [{
type: 'directive',
name: 'Dir',
selector: '[dir]',
exportAs: ['dir'],
inputs: {'dirInput': 'dirInput'},
outputs: {'outputField': 'dirOutput'},
hasNgTemplateContextGuard: true,
}];
const TEMPLATE =
`<div dir #ref="dir">{{ref.value}}</div><ng-template #ref2></ng-template>{{ref2.value2}}`;
it('should trace references to a directive when enabled', () => {
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain('(_t2).value');
});
it('should trace references to an <ng-template> when enabled', () => {
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain('((null as any as core.TemplateRef<any>)).value2');
});
it('should use any for reference types when disabled', () => {
const DISABLED_CONFIG:
TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfNonDomReferences: false};
const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG);
expect(block).toContain('(null as any).value');
});
});
describe('config.checkTypeOfAttributes', () => { describe('config.checkTypeOfAttributes', () => {
const TEMPLATE = `<textarea dir disabled cols="3" [rows]="2">{{ref.value}}</textarea>`; const TEMPLATE = `<textarea dir disabled cols="3" [rows]="2">{{ref.value}}</textarea>`;
const DIRECTIVES: TestDeclaration[] = [{ const DIRECTIVES: TestDeclaration[] = [{