Skip to content

Commit 03db2ae

Browse files
JeanMechethePunderWoman
JeanMeche
authored and
thePunderWoman
committed
fix(compiler): throw on duplicate input/outputs
inputs & outputs cannot be binded to 2 different directives/components properties Eg ``` data = model(); dataChange = output(); // throws because model already emits on the `dataChange` output userSomething = input({alias 'user'}); user = input(); // throws because userSomething already binds to the `user` input ```` fixes #65844 BREAKING CHANGE: The compiler will throw when there a when inputs, outputs or model are binding to the same input/outputs.
1 parent 3bc095d commit 03db2ae

File tree

8 files changed

+42
-11
lines changed

8 files changed

+42
-11
lines changed

‎goldens/public-api/compiler-cli/error_code.api.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ export enum ErrorCode {
4949
DIRECTIVE_INHERITS_UNDECORATED_CTOR = 2006,
5050
// (undocumented)
5151
DIRECTIVE_MISSING_SELECTOR = 2004,
52+
DUPLICATE_BINDING_NAME = 1054,
5253
// (undocumented)
5354
DUPLICATE_DECORATED_PROPERTIES = 1012,
5455
DUPLICATE_VARIABLE_DECLARATION = 8006,

‎packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1346,6 +1346,7 @@ function parseInputFields(
13461346
emitDeclarationOnly: boolean,
13471347
): Record<string, InputMapping> {
13481348
const inputs = {} as Record<string, InputMapping>;
1349+
const bindings = new Map<string, ClassMember>();
13491350

13501351
for (const member of members) {
13511352
const classPropertyName = member.name;
@@ -1364,6 +1365,18 @@ function parseInputFields(
13641365
continue;
13651366
}
13661367

1368+
const bindingPropertyName = inputMapping.bindingPropertyName;
1369+
if (bindings.has(bindingPropertyName)) {
1370+
const firstMember = bindings.get(bindingPropertyName)!;
1371+
throw new FatalDiagnosticError(
1372+
ErrorCode.DUPLICATE_BINDING_NAME,
1373+
member.node ?? clazz,
1374+
`Input '${bindingPropertyName}' is bound to both '${firstMember.name}' and '${member.name}'.`,
1375+
[makeRelatedInformation(firstMember.node ?? clazz, `The first binding is declared here.`)],
1376+
);
1377+
}
1378+
bindings.set(bindingPropertyName, member);
1379+
13671380
if (member.isStatic) {
13681381
throw new FatalDiagnosticError(
13691382
ErrorCode.INCORRECTLY_DECLARED_ON_STATIC_MEMBER,
@@ -1742,6 +1755,7 @@ function parseOutputFields(
17421755
outputsFromMeta: Record<string, string>,
17431756
): Record<string, string> {
17441757
const outputs = {} as Record<string, string>;
1758+
const bindings = new Map<string, ClassMember>();
17451759

17461760
for (const member of members) {
17471761
const decoratorOutput = tryParseDecoratorOutput(member, evaluator, isCore);
@@ -1786,6 +1800,17 @@ function parseOutputFields(
17861800
continue;
17871801
}
17881802

1803+
if (bindings.has(bindingPropertyName)) {
1804+
const firstMember = bindings.get(bindingPropertyName)!;
1805+
throw new FatalDiagnosticError(
1806+
ErrorCode.DUPLICATE_BINDING_NAME,
1807+
member.node ?? clazz,
1808+
`Output '${bindingPropertyName}' is bound to both '${firstMember.name}' and '${member.name}'.`,
1809+
[makeRelatedInformation(firstMember.node ?? clazz, `The first binding is declared here.`)],
1810+
);
1811+
}
1812+
bindings.set(bindingPropertyName, member);
1813+
17891814
// Validate that initializer-based outputs are not accidentally declared
17901815
// in the `outputs` class metadata.
17911816
if (

‎packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ export enum ErrorCode {
5252
*/
5353
INITIALIZER_API_DISALLOWED_MEMBER_VISIBILITY = 1053,
5454

55+
/**
56+
* Raised whenever there are duplicate binding property names for outputs, inputs & models.
57+
*/
58+
DUPLICATE_BINDING_NAME = 1054,
59+
5560
/**
5661
* An Angular feature, like inputs, outputs or queries is incorrectly
5762
* declared on a static member.

‎packages/compiler-cli/test/compliance/test_cases/output_function/GOLDEN_PARTIAL.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ export class TestDir {
8484
clickDecorator2 = new EventEmitter();
8585
_blaDecorator = new EventEmitter();
8686
static ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: TestDir, deps: [], target: i0.ɵɵFactoryTarget.Directive });
87-
static ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: TestDir, isStandalone: true, outputs: { click1: "click1", click2: "click2", click3: "click3", _bla: "decoratorPublicName", _bla2: "decoratorPublicName2", clickDecorator1: "clickDecorator1", clickDecorator2: "clickDecorator2", _blaDecorator: "decoratorPublicName" }, ngImport: i0 });
87+
static ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: TestDir, isStandalone: true, outputs: { click1: "click1", click2: "click2", click3: "click3", _bla: "decoratorPublicName", _bla2: "decoratorPublicName2", clickDecorator1: "clickDecorator1", clickDecorator2: "clickDecorator2", _blaDecorator: "decoratorPublicName3" }, ngImport: i0 });
8888
}
8989
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: TestDir, decorators: [{
9090
type: Directive
@@ -94,7 +94,7 @@ i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDE
9494
type: Output
9595
}], _blaDecorator: [{
9696
type: Output,
97-
args: ['decoratorPublicName']
97+
args: ['decoratorPublicName3']
9898
}] } });
9999

100100
/****************************************************************************************************
@@ -112,6 +112,6 @@ export declare class TestDir {
112112
clickDecorator2: EventEmitter<boolean>;
113113
_blaDecorator: EventEmitter<void>;
114114
static ɵfac: i0.ɵɵFactoryDeclaration<TestDir, never>;
115-
static ɵdir: i0.ɵɵDirectiveDeclaration<TestDir, never, never, {}, { "click1": "click1"; "click2": "click2"; "click3": "click3"; "_bla": "decoratorPublicName"; "_bla2": "decoratorPublicName2"; "clickDecorator1": "clickDecorator1"; "clickDecorator2": "clickDecorator2"; "_blaDecorator": "decoratorPublicName"; }, never, never, true, never>;
115+
static ɵdir: i0.ɵɵDirectiveDeclaration<TestDir, never, never, {}, { "click1": "click1"; "click2": "click2"; "click3": "click3"; "_bla": "decoratorPublicName"; "_bla2": "decoratorPublicName2"; "clickDecorator1": "clickDecorator1"; "clickDecorator2": "clickDecorator2"; "_blaDecorator": "decoratorPublicName3"; }, never, never, true, never>;
116116
}
117117

‎packages/compiler-cli/test/compliance/test_cases/output_function/mixed_variants.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ export class TestDir {
1010
_bla2: "decoratorPublicName2",
1111
clickDecorator1: "clickDecorator1",
1212
clickDecorator2: "clickDecorator2",
13-
_blaDecorator: "decoratorPublicName"
13+
_blaDecorator: "decoratorPublicName3"
1414
}
1515
…
1616
});

‎packages/compiler-cli/test/compliance/test_cases/output_function/mixed_variants.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,5 @@ export class TestDir {
1111

1212
@Output() clickDecorator1 = new EventEmitter();
1313
@Output() clickDecorator2 = new EventEmitter<boolean>();
14-
@Output('decoratorPublicName') _blaDecorator = new EventEmitter<void>();
14+
@Output('decoratorPublicName3') _blaDecorator = new EventEmitter<void>();
1515
}

‎packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,8 @@ runInEachFileSystem(() => {
155155
expect(diags[0].code).toBeGreaterThan(0);
156156
});
157157

158-
it('should produce diagnostics when mapping to multiple fields and bound types are incorrect', () => {
158+
// This is not supported at runtime
159+
xit('should produce diagnostics when mapping to multiple fields and bound types are incorrect', () => {
159160
env.tsconfig({
160161
fullTemplateTypeCheck: true,
161162
strictInputTypes: true,
@@ -241,7 +242,8 @@ runInEachFileSystem(() => {
241242
);
242243
});
243244

244-
it('should support one input property mapping to multiple fields', () => {
245+
/** This is not supported at runtime */
246+
xit('should support one input property mapping to multiple fields', () => {
245247
env.write(
246248
'test.ts',
247249
`

‎packages/language-service/test/grp1/diagnostic_spec.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ describe('getSemanticDiagnostics', () => {
5959
expect(messageText).toBe(`Property 'nope' does not exist on type 'AppComponent'.`);
6060
});
6161

62-
it('produces diagnostic for duplicate docarated property rather than crashing', () => {
62+
it('produces diagnostic for duplicate decorated property rather than crashing', () => {
6363
const files = {
6464
'app.ts': `
6565
import {Component, Input} from '@angular/core';
@@ -82,9 +82,7 @@ describe('getSemanticDiagnostics', () => {
8282
expect(diags[0].messageText).toBe(`Duplicate identifier 'test1'.`);
8383
expect(diags[1].category).toBe(ts.DiagnosticCategory.Error);
8484
expect(diags[1].file?.fileName).toBe('/test/app.ts');
85-
expect(diags[1].messageText).toBe(
86-
`Duplicate decorated properties found on class 'AppComponent': test1`,
87-
);
85+
expect(diags[1].messageText).toBe(`Input 'test1' is bound to both 'test1' and 'test1'.`);
8886
});
8987

9088
it('should process external template', () => {

0 commit comments

Comments
 (0)