Skip to content

Commit 69a8399

Browse files
devversionatscott
authored andcommitted
fix(compiler-cli): do not throw when retrieving TCB symbol for signal input with restricted access (#55774)
Currently when attempting to retrieve a TCB symbol for an input binding that refers to a signal input with e.g. `protected`, while the `honorAccessModifiersForInputBindings` flag is `false`, Angular will throw a runtime exception because the symbol retrieval code always expects a proper field access in the TCB. This is not the case with `honorAccessModifiersForInputBindings = false`, as TCB will allocate a temporary variable when ignoring the field access. This will then trigger the runtime exception (which we added to flag such "unexpected" cases). This commit handles it gracefully, as it's valid TCB, but we simply cannot generate a proper TCB symbol (yet). This is similar to `@Input` decorator inputs. In the future we may implement logic to build up TCB symbols for non-property access bindings, for both signal inputs or `@Input` inputs. This commit just avoids a build exception. Related to: #54324. PR Close #55774
1 parent 6025424 commit 69a8399

File tree

2 files changed

+86
-8
lines changed

2 files changed

+86
-8
lines changed

packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,7 @@ export class SymbolBuilder {
416416
}
417417

418418
const signalInputAssignment = unwrapSignalInputWriteTAccessor(node.left);
419+
let fieldAccessExpr: ts.PropertyAccessExpression | ts.ElementAccessExpression;
419420
let symbolInfo: TsNodeSymbolInfo | null = null;
420421

421422
// Signal inputs need special treatment because they are generated with an extra keyed
@@ -424,9 +425,18 @@ export class SymbolBuilder {
424425
// - The definition symbol of the input should be the input class member, and not the
425426
// internal write accessor. Symbol should resolve `_t1.prop`.
426427
if (signalInputAssignment !== null) {
428+
// Note: If the field expression for the input binding refers to just an identifier,
429+
// then we are handling the case of a temporary variable being used for the input field.
430+
// This is the case with `honorAccessModifiersForInputBindings = false` and in those cases
431+
// we cannot resolve the owning directive, similar to how we guard above with `isAccessExpression`.
432+
if (ts.isIdentifier(signalInputAssignment.fieldExpr)) {
433+
continue;
434+
}
435+
427436
const fieldSymbol = this.getSymbolOfTsNode(signalInputAssignment.fieldExpr);
428437
const typeSymbol = this.getSymbolOfTsNode(signalInputAssignment.typeExpr);
429438

439+
fieldAccessExpr = signalInputAssignment.fieldExpr;
430440
symbolInfo =
431441
fieldSymbol === null || typeSymbol === null
432442
? null
@@ -436,17 +446,15 @@ export class SymbolBuilder {
436446
tsType: typeSymbol.tsType,
437447
};
438448
} else {
449+
fieldAccessExpr = node.left;
439450
symbolInfo = this.getSymbolOfTsNode(node.left);
440451
}
441452

442453
if (symbolInfo === null || symbolInfo.tsSymbol === null) {
443454
continue;
444455
}
445456

446-
const target = this.getDirectiveSymbolForAccessExpression(
447-
signalInputAssignment?.fieldExpr ?? node.left,
448-
consumer,
449-
);
457+
const target = this.getDirectiveSymbolForAccessExpression(fieldAccessExpr, consumer);
450458
if (target === null) {
451459
continue;
452460
}
@@ -771,7 +779,7 @@ function sourceSpanEqual(a: ParseSourceSpan, b: ParseSourceSpan) {
771779
}
772780

773781
function unwrapSignalInputWriteTAccessor(expr: ts.LeftHandSideExpression): null | {
774-
fieldExpr: ts.ElementAccessExpression | ts.PropertyAccessExpression;
782+
fieldExpr: ts.ElementAccessExpression | ts.PropertyAccessExpression | ts.Identifier;
775783
typeExpr: ts.ElementAccessExpression;
776784
} {
777785
// e.g. `_t2.inputA[i2.ɵINPUT_SIGNAL_BRAND_WRITE_TYPE]`
@@ -793,11 +801,15 @@ function unwrapSignalInputWriteTAccessor(expr: ts.LeftHandSideExpression): null
793801
return null;
794802
}
795803

796-
// Assert that the `_t2.inputA` is actually either a keyed element access, or
797-
// property access expression. This is checked for type safety and to catch unexpected cases.
804+
// Assert that the expression is either:
805+
// - `_t2.inputA[ɵINPUT_SIGNAL_BRAND_WRITE_TYPE]` or (common case)
806+
// - or `_t2['input-A'][ɵINPUT_SIGNAL_BRAND_WRITE_TYPE]` (non-identifier input field names)
807+
// - or `_dirInput[ɵINPUT_SIGNAL_BRAND_WRITE_TYPE` (honorAccessModifiersForInputBindings=false)
808+
// This is checked for type safety and to catch unexpected cases.
798809
if (
799810
!ts.isPropertyAccessExpression(expr.expression) &&
800-
!ts.isElementAccessExpression(expr.expression)
811+
!ts.isElementAccessExpression(expr.expression) &&
812+
!ts.isIdentifier(expr.expression)
801813
) {
802814
throw new Error('Unexpected expression for signal input write type.');
803815
}

packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1294,6 +1294,72 @@ runInEachFileSystem(() => {
12941294
).toEqual('inputB');
12951295
});
12961296

1297+
// Note that `honorAccessModifiersForInputBindings` is `false` even with `--strictTemplates`,
1298+
// so this captures a potential common scenario, assuming the input is restricted.
1299+
it('should not throw when retrieving a symbol for a signal-input with restricted access', () => {
1300+
const fileName = absoluteFrom('/main.ts');
1301+
const dirFile = absoluteFrom('/dir.ts');
1302+
const templateString = `
1303+
@if (true) {
1304+
<div dir [inputA]="'ok'"></div>
1305+
}
1306+
`;
1307+
const {program, templateTypeChecker} = setup(
1308+
[
1309+
{
1310+
fileName,
1311+
templates: {'Cmp': templateString},
1312+
declarations: [
1313+
{
1314+
name: 'TestDir',
1315+
selector: '[dir]',
1316+
file: dirFile,
1317+
type: 'directive',
1318+
restrictedInputFields: ['inputA'],
1319+
inputs: {
1320+
inputA: {
1321+
bindingPropertyName: 'inputA',
1322+
isSignal: true,
1323+
classPropertyName: 'inputA',
1324+
required: false,
1325+
transform: null,
1326+
},
1327+
},
1328+
},
1329+
],
1330+
},
1331+
{
1332+
fileName: dirFile,
1333+
source: `
1334+
import {InputSignal} from '@angular/core';
1335+
1336+
export class TestDir {
1337+
protected inputA: InputSignal<string> = null!;
1338+
}
1339+
`,
1340+
templates: {},
1341+
},
1342+
],
1343+
{honorAccessModifiersForInputBindings: false},
1344+
);
1345+
const sf = getSourceFileOrError(program, fileName);
1346+
const cmp = getClass(sf, 'Cmp');
1347+
1348+
const nodes = templateTypeChecker.getTemplate(cmp)!;
1349+
const ifNode = nodes[0] as TmplAstIfBlock;
1350+
const ifBranchNode = ifNode.branches[0];
1351+
const testElement = ifBranchNode.children[0] as TmplAstElement;
1352+
1353+
const inputAbinding = testElement.inputs[0];
1354+
const aSymbol = templateTypeChecker.getSymbolOfNode(inputAbinding, cmp);
1355+
expect(aSymbol)
1356+
.withContext(
1357+
'Symbol builder does not return symbols for restricted inputs with ' +
1358+
'`honorAccessModifiersForInputBindings = false` (same for decorator inputs)',
1359+
)
1360+
.toBe(null);
1361+
});
1362+
12971363
it('does not retrieve a symbol for an input when undeclared', () => {
12981364
const fileName = absoluteFrom('/main.ts');
12991365
const dirFile = absoluteFrom('/dir.ts');

0 commit comments

Comments
 (0)