Skip to content

Commit

Permalink
feat(ivy): add flag to disable checking of text attributes (#33365)
Browse files Browse the repository at this point in the history
For elements that have a text attribute, it may happen that the element
is matched by a directive that consumes the attribute as an input. In
that case, the template type checker will validate the correctness of
the attribute with respect to the directive's declared type of the
input, which would typically be `boolean` for the `disabled` input.
Since empty attributes are assigned the empty string at runtime, the
template type checker would report an error for this template.

This commit introduces a strictness flag to help alleviate this
particular situation, effectively ignoring text attributes that happen
to be consumed by a directive.

PR Close #33365
  • Loading branch information
JoostK authored and AndrewKushnir committed Oct 24, 2019
1 parent 4aa51b7 commit d8ce212
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 0 deletions.
2 changes: 2 additions & 0 deletions packages/compiler-cli/src/ngtsc/program.ts
Expand Up @@ -431,6 +431,7 @@ export class NgtscProgram implements api.Program {
checkTemplateBodies: true,
checkTypeOfInputBindings: true,
strictNullInputBindings: true,
checkTypeOfAttributes: true,
// Even in full template type-checking mode, DOM binding checks are not quite ready yet.
checkTypeOfDomBindings: false,
checkTypeOfOutputEvents: true,
Expand All @@ -451,6 +452,7 @@ export class NgtscProgram implements api.Program {
checkTemplateBodies: false,
checkTypeOfInputBindings: false,
strictNullInputBindings: false,
checkTypeOfAttributes: false,
checkTypeOfDomBindings: false,
checkTypeOfOutputEvents: false,
checkTypeOfAnimationEvents: false,
Expand Down
15 changes: 15 additions & 0 deletions packages/compiler-cli/src/ngtsc/typecheck/src/api.ts
Expand Up @@ -98,9 +98,24 @@ export interface TypeCheckingConfig {
* binding expressions are wrapped in a non-null assertion operator to effectively disable strict
* null checks. This may be particularly useful when the directive is from a library that is not
* compiled with `strictNullChecks` enabled.
*
* If `checkTypeOfInputBindings` is set to `false`, this flag has no effect.
*/
strictNullInputBindings: boolean;

/**
* Whether to check text attributes that happen to be consumed by a directive or component.
*
* For example, in a template containing `<input matInput disabled>` the `disabled` attribute ends
* up being consumed as an input with type `boolean` by the `matInput` directive. At runtime the
* input will be set to the attribute's string value, which is the empty string for attributes
* without a value, so with this flag set to `true` an error would be reported. If set to `false`,
* text attributes will never report an error.
*
* Note that if `checkTypeOfInputBindings` is set to `false`, this flag has no effect.
*/
checkTypeOfAttributes: boolean;

/**
* Whether to check the left-hand side type of binding operations to DOM properties.
*
Expand Down
Expand Up @@ -1153,6 +1153,11 @@ function tcbGetDirectiveInputs(
return;
}

// Skip text attributes if configured to do so.
if (!tcb.env.config.checkTypeOfAttributes && attr instanceof TmplAstTextAttribute) {
return;
}

// Skip the attribute if the directive does not have an input for it.
if (!propMatch.has(attr.name)) {
return;
Expand Down
19 changes: 19 additions & 0 deletions packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts
Expand Up @@ -154,6 +154,25 @@ runInEachFileSystem(() => {
]);
});

it('checks text attributes that are consumed by bindings with literal string types', () => {
const messages = diagnose(
`<div dir mode="drak"></div><div dir mode="light"></div>`, `
class Dir {
mode: 'dark'|'light';
}
class TestComponent {}`,
[{
type: 'directive',
name: 'Dir',
selector: '[dir]',
inputs: {'mode': 'mode'},
}]);

expect(messages).toEqual([
`synthetic.html(1, 10): Type '"drak"' is not assignable to type '"dark" | "light"'.`,
]);
});

it('produces diagnostics for pipes', () => {
const messages = diagnose(
`<div>{{ person.name | pipe:person.age:1 }}</div>`, `
Expand Down
2 changes: 2 additions & 0 deletions packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts
Expand Up @@ -149,6 +149,7 @@ export const ALL_ENABLED_CONFIG: TypeCheckingConfig = {
checkTemplateBodies: true,
checkTypeOfInputBindings: true,
strictNullInputBindings: true,
checkTypeOfAttributes: true,
// Feature is still in development.
// TODO(alxhub): enable when DOM checking via lib.dom.d.ts is further along.
checkTypeOfDomBindings: false,
Expand Down Expand Up @@ -201,6 +202,7 @@ export function tcb(
checkQueries: false,
checkTypeOfInputBindings: true,
strictNullInputBindings: true,
checkTypeOfAttributes: true,
checkTypeOfDomBindings: false,
checkTypeOfOutputEvents: true,
checkTypeOfAnimationEvents: true,
Expand Down
Expand Up @@ -293,6 +293,7 @@ describe('type check blocks', () => {
checkTemplateBodies: true,
checkTypeOfInputBindings: true,
strictNullInputBindings: true,
checkTypeOfAttributes: true,
checkTypeOfDomBindings: false,
checkTypeOfOutputEvents: true,
checkTypeOfAnimationEvents: true,
Expand Down Expand Up @@ -438,6 +439,31 @@ describe('type check blocks', () => {
});
});

describe('config.checkTypeOfAttributes', () => {
const TEMPLATE = `<textarea dir disabled cols="3" [rows]="2">{{ref.value}}</textarea>`;
const DIRECTIVES: TestDeclaration[] = [{
type: 'directive',
name: 'Dir',
selector: '[dir]',
inputs: {'disabled': 'disabled', 'cols': 'cols', 'rows': 'rows'},
}];

it('should assign string value to the input when enabled', () => {
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain('disabled: ("")');
expect(block).toContain('cols: ("3")');
expect(block).toContain('rows: (2)');
});

it('should use any for attributes but still check bound attributes when disabled', () => {
const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfAttributes: false};
const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG);
expect(block).toContain('disabled: (null as any)');
expect(block).toContain('cols: (null as any)');
expect(block).toContain('rows: (2)');
});
});

describe('config.checkTypeOfPipes', () => {
const TEMPLATE = `{{a | test:b:c}}`;
const PIPES: TestDeclaration[] = [{
Expand Down

0 comments on commit d8ce212

Please sign in to comment.