Skip to content

Commit

Permalink
fix(ivy): type-checking of properties which map to multiple fields (#…
Browse files Browse the repository at this point in the history
…34649)

It's possible to declare multiple inputs for a directive/component which all
map to the same property name. This is usually done in error, as only one of
any bindings to the property will "win".

In the template type-checker, an error was previously being raised as a
result of this ambiguity. Specifically, a type constructor was produced
which required a binding for each field, but only one of the fields had
a value via the binding. TypeScript would (rightfully) error on missing
values for the remaining fields. This ultimately was happening when the
code which generated the default values for "unset" inputs belonging to
directives or pipes used the final mapping from properties to fields as
a source for field names.

Instead, this commit uses the original list of fields to generate unset
input values, which correctly provides values for fields which shared a
property name but didn't receive the final binding.

PR Close #34649
  • Loading branch information
alxhub authored and AndrewKushnir committed Jan 23, 2020
1 parent 4eeb6cf commit b04d4ba
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 1 deletion.
Expand Up @@ -1187,7 +1187,10 @@ function tcbGetDirectiveInputs(

// To determine which of directive's inputs are unset, we keep track of the set of field names
// that have not been seen yet. A field is removed from this set once a binding to it is found.
const unsetFields = new Set(propMatch.values());
// Note: it's actually important here that `propMatch.values()` isn't used, as there can be
// multiple fields which share the same property name and only one of them will be listed as a
// value in `propMatch`.
const unsetFields = new Set(Object.keys(inputs));

el.inputs.forEach(processAttribute);
el.attributes.forEach(processAttribute);
Expand Down
27 changes: 27 additions & 0 deletions packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
Expand Up @@ -174,6 +174,33 @@ export declare class AnimationEvent {
.toEqual(`Argument of type 'string' is not assignable to parameter of type 'number'.`);
});

it('should support one input property mapping to multiple fields', () => {
env.write('test.ts', `
import {Component, Directive, Input, NgModule} from '@angular/core';
@Directive({
selector: '[dir]',
})
export class Dir {
@Input('propertyName') fieldA!: string;
@Input('propertyName') fieldB!: string;
}
@Component({
selector: 'test-cmp',
template: '<div dir propertyName="test"></div>',
})
export class Cmp {}
@NgModule({declarations: [Dir, Cmp]})
export class Module {}
`);

const diags = env.driveDiagnostics();
expect(diags.length).toBe(0);
});

it('should check event bindings', () => {
env.tsconfig({fullTemplateTypeCheck: true, strictOutputEventTypes: true});
env.write('test.ts', `
Expand Down

0 comments on commit b04d4ba

Please sign in to comment.