Skip to content

Commit b2066d4

Browse files
devversionatscott
authored andcommitted
refactor(compiler-cli): detect input functions without partial evaluation (angular#53872)
This allows us to ensure signal inputs and a potential JIT transform remain single file compilation compatible. The consequences are that options need to be statically analyzable more strictly, compared to loosened restrictions with static interpretation where e.g. `alias` can be defined through a shared variable. PR Close angular#53872
1 parent f6a32c0 commit b2066d4

File tree

7 files changed

+80
-57
lines changed

7 files changed

+80
-57
lines changed

packages/compiler-cli/private/tooling.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,7 @@ export function angularJitApplicationTransform(
6161
typeChecker, reflectionHost, [], isCore,
6262
/* enableClosureCompiler */ false);
6363

64-
const inputSignalMetadataTransform =
65-
getInputSignalsMetadataTransform(reflectionHost, evaluator, isCore);
64+
const inputSignalMetadataTransform = getInputSignalsMetadataTransform(reflectionHost, isCore);
6665

6766
return (ctx) => {
6867
return (sourceFile) => {

packages/compiler-cli/src/ngtsc/annotations/directive/src/input_function.ts

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@
88

99
import ts from 'typescript';
1010

11+
import {ErrorCode, FatalDiagnosticError} from '../../../diagnostics';
1112
import {InputMapping} from '../../../metadata';
12-
import {PartialEvaluator} from '../../../partial_evaluator';
13-
import {ClassMember, ReflectionHost} from '../../../reflection';
13+
import {ClassMember, ReflectionHost, reflectObjectLiteral} from '../../../reflection';
1414

1515
/** Metadata describing an input declared via the `input` function. */
1616
interface InputMemberMetadata {
@@ -45,8 +45,7 @@ function tryParseInputInitializerAndOptions(
4545
}
4646

4747
// Case 1: No `required`.
48-
// TODO(signal-input-public): Clean this up.
49-
if (target.text === 'input' || target.text === 'ɵinput') {
48+
if (target.text === 'input') {
5049
if (!isReferenceToInputFunction(target, coreModule, reflector)) {
5150
return null;
5251
}
@@ -108,8 +107,32 @@ function isReferenceToInputFunction(
108107
targetImport = {name: target.text};
109108
}
110109

111-
// TODO(signal-input-public): Clean this up.
112-
return targetImport.name === 'input' || targetImport.name === 'ɵinput';
110+
return targetImport.name === 'input';
111+
}
112+
113+
/** Parses and validates the input function options expression. */
114+
function parseAndValidateOptions(optionsNode: ts.Expression): {alias: string|undefined} {
115+
if (!ts.isObjectLiteralExpression(optionsNode)) {
116+
throw new FatalDiagnosticError(
117+
ErrorCode.VALUE_HAS_WRONG_TYPE, optionsNode,
118+
'Argument needs to be an object literal that is statically analyzable.');
119+
}
120+
121+
const options = reflectObjectLiteral(optionsNode);
122+
let alias: string|undefined = undefined;
123+
124+
if (options.has('alias')) {
125+
const aliasExpr = options.get('alias')!;
126+
if (!ts.isStringLiteralLike(aliasExpr)) {
127+
throw new FatalDiagnosticError(
128+
ErrorCode.VALUE_HAS_WRONG_TYPE, aliasExpr,
129+
'Alias needs to be a string that is statically analyzable.');
130+
}
131+
132+
alias = aliasExpr.text;
133+
}
134+
135+
return {alias};
113136
}
114137

115138
/**
@@ -118,25 +141,20 @@ function isReferenceToInputFunction(
118141
*/
119142
export function tryParseSignalInputMapping(
120143
member: Pick<ClassMember, 'name'|'value'>, reflector: ReflectionHost,
121-
evaluator: PartialEvaluator, coreModule: string|undefined): InputMapping|null {
144+
coreModule: string|undefined): InputMapping|null {
122145
const signalInput = tryParseInputInitializerAndOptions(member, reflector, coreModule);
123146
if (signalInput === null) {
124147
return null;
125148
}
126149

127150
const optionsNode = signalInput.optionsNode;
128-
const options = optionsNode !== undefined ? evaluator.evaluate(optionsNode) : null;
151+
const options = optionsNode !== undefined ? parseAndValidateOptions(optionsNode) : null;
129152
const classPropertyName = member.name;
130153

131-
let bindingPropertyName = classPropertyName;
132-
if (options instanceof Map && typeof options.get('alias') === 'string') {
133-
bindingPropertyName = options.get('alias') as string;
134-
}
135-
136154
return {
137155
isSignal: true,
138156
classPropertyName,
139-
bindingPropertyName,
157+
bindingPropertyName: options?.alias ?? classPropertyName,
140158
required: signalInput.isRequired,
141159
// Signal inputs do not capture complex transform metadata.
142160
// See more details in the `transform` type of `InputMapping`.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -718,7 +718,7 @@ function tryParseInputFieldMapping(
718718
const classPropertyName = member.name;
719719

720720
const decorator = tryGetDecoratorOnMember(member, 'Input', coreModule);
721-
const signalInputMapping = tryParseSignalInputMapping(member, reflector, evaluator, coreModule);
721+
const signalInputMapping = tryParseSignalInputMapping(member, reflector, coreModule);
722722

723723
if (decorator !== null && signalInputMapping !== null) {
724724
throw new FatalDiagnosticError(

packages/compiler-cli/src/transformers/jit_transforms/signal_inputs_metadata_transform.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ const coreModuleName = '@angular/core';
3434
*/
3535
export function getInputSignalsMetadataTransform(
3636
host: ReflectionHost,
37-
evaluator: PartialEvaluator,
3837
isCore: boolean,
3938
): ts.TransformerFactory<ts.SourceFile> {
4039
return (ctx) => {
@@ -43,7 +42,7 @@ export function getInputSignalsMetadataTransform(
4342

4443
sourceFile = ts.visitNode(
4544
sourceFile,
46-
createTransformVisitor(ctx, host, evaluator, importManager, isCore),
45+
createTransformVisitor(ctx, host, importManager, isCore),
4746
ts.isSourceFile,
4847
);
4948

@@ -64,7 +63,6 @@ export function getInputSignalsMetadataTransform(
6463
function createTransformVisitor(
6564
ctx: ts.TransformationContext,
6665
host: ReflectionHost,
67-
evaluator: PartialEvaluator,
6866
importManager: ImportManager,
6967
isCore: boolean,
7068
): ts.Visitor<ts.Node, ts.Node> {
@@ -74,8 +72,7 @@ function createTransformVisitor(
7472
(d) => decoratorsWithInputs.some((name) => isAngularDecorator(d, name, isCore)));
7573

7674
if (angularDecorator !== undefined) {
77-
return visitClassDeclaration(
78-
ctx, host, evaluator, importManager, node, angularDecorator, isCore);
75+
return visitClassDeclaration(ctx, host, importManager, node, angularDecorator, isCore);
7976
}
8077
}
8178

@@ -88,7 +85,6 @@ function createTransformVisitor(
8885
function visitClassDeclaration(
8986
ctx: ts.TransformationContext,
9087
host: ReflectionHost,
91-
evaluator: PartialEvaluator,
9288
importManager: ImportManager,
9389
clazz: ts.ClassDeclaration,
9490
classDecorator: Decorator,
@@ -110,7 +106,6 @@ function visitClassDeclaration(
110106
const inputMapping = tryParseSignalInputMapping(
111107
{name: member.name.text, value: member.initializer ?? null},
112108
host,
113-
evaluator,
114109
isCore ? coreModuleName : undefined,
115110
);
116111
if (inputMapping === null) {

packages/compiler-cli/test/ngtsc/authoring_spec.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,48 @@ runInEachFileSystem(() => {
7070
]);
7171
});
7272

73+
it('should fail if signal input declares a non-statically analyzable alias', () => {
74+
env.write('test.ts', `
75+
import {Directive, input} from '@angular/core';
76+
77+
const ALIAS = 'bla';
78+
79+
@Directive({
80+
inputs: ['data'],
81+
})
82+
export class TestDir {
83+
data = input('test', {alias: ALIAS});
84+
}
85+
`);
86+
const diagnostics = env.driveDiagnostics();
87+
expect(diagnostics).toEqual([
88+
jasmine.objectContaining({
89+
messageText: 'Alias needs to be a string that is statically analyzable.',
90+
}),
91+
]);
92+
});
93+
94+
it('should fail if signal input declares a non-statically analyzable options', () => {
95+
env.write('test.ts', `
96+
import {Directive, input} from '@angular/core';
97+
98+
const OPTIONS = {};
99+
100+
@Directive({
101+
inputs: ['data'],
102+
})
103+
export class TestDir {
104+
data = input('test', OPTIONS);
105+
}
106+
`);
107+
const diagnostics = env.driveDiagnostics();
108+
expect(diagnostics).toEqual([
109+
jasmine.objectContaining({
110+
messageText: 'Argument needs to be an object literal that is statically analyzable.',
111+
}),
112+
]);
113+
});
114+
73115
it('should fail if signal input is declared on static member', () => {
74116
env.write('test.ts', `
75117
import {Directive, input} from '@angular/core';

packages/compiler-cli/test/signal_inputs_metadata_transform_spec.ts

Lines changed: 1 addition & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
import ts from 'typescript';
1010

11-
import {PartialEvaluator} from '../src/ngtsc/partial_evaluator';
1211
import {TypeScriptReflectionHost} from '../src/ngtsc/reflection';
1312
import {getDownlevelDecoratorsTransform, getInputSignalsMetadataTransform} from '../src/transformers/jit_transforms';
1413

@@ -51,10 +50,9 @@ describe('signal inputs metadata transform', () => {
5150
const testFile = program.getSourceFile(TEST_FILE_INPUT);
5251
const typeChecker = program.getTypeChecker();
5352
const reflectionHost = new TypeScriptReflectionHost(typeChecker);
54-
const evaluator = new PartialEvaluator(reflectionHost, typeChecker, null);
5553
const transformers: ts.CustomTransformers = {
5654
before: [
57-
getInputSignalsMetadataTransform(reflectionHost, evaluator, /* isCore */ false),
55+
getInputSignalsMetadataTransform(reflectionHost, /* isCore */ false),
5856
]
5957
};
6058

@@ -178,34 +176,6 @@ describe('signal inputs metadata transform', () => {
178176
`));
179177
});
180178

181-
it('should add `@Input` decorator for signal inputs with alias options, resolving ' +
182-
'references in input options',
183-
() => {
184-
const result = transform(`
185-
import {input, Directive} from '@angular/core';
186-
187-
const aliases = {
188-
a: 'public1',
189-
b: 'public2',
190-
}
191-
192-
@Directive({})
193-
class MyDir {
194-
someInput = input(null, {alias: aliases.a});
195-
someInput2 = input.required<string>({alias: aliases.b});
196-
}
197-
`);
198-
199-
expect(result).toContain(omitLeadingWhitespace(`
200-
__decorate([
201-
i0.Input({ isSignal: true, alias: "public1", required: false, transform: undefined })
202-
], MyDir.prototype, "someInput", void 0);
203-
__decorate([
204-
i0.Input({ isSignal: true, alias: "public2", required: true, transform: undefined })
205-
], MyDir.prototype, "someInput2", void 0);
206-
`));
207-
});
208-
209179
it('should not transform signal inputs with an existing `@Input` decorator', () => {
210180
// This is expected to not happen because technically the TS code for signal inputs
211181
// should never discover both `@Input` and signal inputs. We handle this gracefully

packages/core/test/acceptance/input_signals/signal_inputs_test_compiler.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ async function main() {
3030
const evaluator = new PartialEvaluator(host, program.getTypeChecker(), null);
3131
const outputFile = ts.transform(
3232
program.getSourceFile(inputTsExecPath)!,
33-
[getInputSignalsMetadataTransform(host, evaluator, /* isCore */ false)],
34-
program.getCompilerOptions());
33+
[getInputSignalsMetadataTransform(host, /* isCore */ false)], program.getCompilerOptions());
3534

3635
await fs.promises.writeFile(
3736
outputExecPath, ts.createPrinter().printFile(outputFile.transformed[0]));

0 commit comments

Comments
 (0)