Skip to content

Commit cfab5a5

Browse files
devversiondevversion
committed
refactor(core): detect signal input transforms independently of flag (#53808)
This commit changes the `HasTransform` flag to be only concerned with decorator inputs. This allows us to automatically detect signal input transforms without reliance on the flag, resulting in less complexity in the compiler (as outlined in the design doc) and various other places, while it also allows us to simplify JIT support for signal inputs because there would be no need to capture the "hasTransform" state in the decorator so that JIT can generate the according input flags. `isSignal` will still persist as an input flag to allow for monomorphic and highly efficient distinguishing at runtime, whether an input is signal based or not. JIT transform will also need to propagate this information to the runtime somehow. PR Close #53808
1 parent eee0af0 commit cfab5a5

9 files changed

Lines changed: 62 additions & 60 deletions

File tree

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_input_outputs/input_transform_definition.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
MyDirective.ɵdir = /*@__PURE__*/ $r3$.ɵɵdefineDirective({
22
…
33
inputs: {
4-
functionDeclarationInput: [$r3$.ɵɵInputFlags.HasTransform, "functionDeclarationInput", "functionDeclarationInput", toNumber],
5-
inlineFunctionInput: [$r3$.ɵɵInputFlags.HasTransform, "inlineFunctionInput", "inlineFunctionInput", (value, _) => value ? 1 : 0]
4+
functionDeclarationInput: [$r3$.ɵɵInputFlags.HasDecoratorInputTransform, "functionDeclarationInput", "functionDeclarationInput", toNumber],
5+
inlineFunctionInput: [$r3$.ɵɵInputFlags.HasDecoratorInputTransform, "inlineFunctionInput", "inlineFunctionInput", (value, _) => value ? 1 : 0]
66
},
77
features: [$r3$.ɵɵInputTransformsFeature]…
88
});

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

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1053,12 +1053,12 @@ runInEachFileSystem(() => {
10531053
const jsContents = env.getContents('test.js');
10541054

10551055
expect(jsContents)
1056-
.toContain('inputs: { x: [i0.ɵɵInputFlags.HasTransform, "x", "x", externalFunc] }');
1056+
.toContain(
1057+
'inputs: { x: [i0.ɵɵInputFlags.HasDecoratorInputTransform, "x", "x", externalFunc] }');
10571058
});
10581059

1059-
it('should generate input info for transform function imported externally using namespace',
1060-
() => {
1061-
env.write('test.ts', `
1060+
it('should generate input info for transform function imported externally using namespace', () => {
1061+
env.write('test.ts', `
10621062
import {Component, NgModule, Input} from '@angular/core';
10631063
import * as n from './some_where';
10641064
@@ -1071,13 +1071,13 @@ runInEachFileSystem(() => {
10711071
}
10721072
`);
10731073

1074-
env.driveMain();
1075-
const jsContents = env.getContents('test.js');
1074+
env.driveMain();
1075+
const jsContents = env.getContents('test.js');
10761076

1077-
expect(jsContents)
1078-
.toContain(
1079-
'inputs: { x: [i0.ɵɵInputFlags.HasTransform, "x", "x", n.externalFunc] }');
1080-
});
1077+
expect(jsContents)
1078+
.toContain(
1079+
'inputs: { x: [i0.ɵɵInputFlags.HasDecoratorInputTransform, "x", "x", n.externalFunc] }');
1080+
});
10811081

10821082
it('should generate input info for transform function defined locally', () => {
10831083
env.write('test.ts', `
@@ -1100,7 +1100,8 @@ runInEachFileSystem(() => {
11001100
const jsContents = env.getContents('test.js');
11011101

11021102
expect(jsContents)
1103-
.toContain('inputs: { x: [i0.ɵɵInputFlags.HasTransform, "x", "x", localFunc] }');
1103+
.toContain(
1104+
'inputs: { x: [i0.ɵɵInputFlags.HasDecoratorInputTransform, "x", "x", localFunc] }');
11041105
});
11051106

11061107
it('should generate input info for inline transform function', () => {
@@ -1121,7 +1122,7 @@ runInEachFileSystem(() => {
11211122

11221123
expect(jsContents)
11231124
.toContain(
1124-
'inputs: { x: [i0.ɵɵInputFlags.HasTransform, "x", "x", (v) => v + \'TRANSFORMED!\'] }');
1125+
'inputs: { x: [i0.ɵɵInputFlags.HasDecoratorInputTransform, "x", "x", (v) => v + \'TRANSFORMED!\'] }');
11251126
});
11261127

11271128
it('should not check inline function param type', () => {
@@ -1142,7 +1143,7 @@ runInEachFileSystem(() => {
11421143

11431144
expect(jsContents)
11441145
.toContain(
1145-
'inputs: { x: [i0.ɵɵInputFlags.HasTransform, "x", "x", v => v + \'TRANSFORMED!\'] }');
1146+
'inputs: { x: [i0.ɵɵInputFlags.HasDecoratorInputTransform, "x", "x", v => v + \'TRANSFORMED!\'] }');
11461147
});
11471148
});
11481149
});

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

Lines changed: 25 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -8605,7 +8605,7 @@ function allTests(os: string) {
86058605

86068606
expect(jsContents)
86078607
.toContain(
8608-
'inputs: { value: [i0.ɵɵInputFlags.HasTransform, "value", "value", toNumber] }');
8608+
'inputs: { value: [i0.ɵɵInputFlags.HasDecoratorInputTransform, "value", "value", toNumber] }');
86098609
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
86108610
expect(dtsContents).toContain('static ngAcceptInputType_value: boolean | string;');
86118611
});
@@ -8629,20 +8629,19 @@ function allTests(os: string) {
86298629

86308630
expect(jsContents)
86318631
.toContain(
8632-
'inputs: { value: [i0.ɵɵInputFlags.HasTransform, "value", "value", toNumber] }');
8632+
'inputs: { value: [i0.ɵɵInputFlags.HasDecoratorInputTransform, "value", "value", toNumber] }');
86338633
expect(jsContents)
86348634
.toContain('features: [i0.ɵɵInputTransformsFeature, i0.ɵɵStandaloneFeature]');
86358635
expect(dtsContents).toContain('static ngAcceptInputType_value: boolean | string;');
86368636
});
86378637

8638-
it('should compile an input with a transform function that contains a generic parameter',
8639-
() => {
8640-
env.write('/types.ts', `
8638+
it('should compile an input with a transform function that contains a generic parameter', () => {
8639+
env.write('/types.ts', `
86418640
export interface GenericWrapper<T> {
86428641
value: T;
86438642
}
86448643
`);
8645-
env.write('/test.ts', `
8644+
env.write('/test.ts', `
86468645
import {Directive, Input} from '@angular/core';
86478646
import {GenericWrapper} from './types';
86488647
@@ -8654,20 +8653,20 @@ function allTests(os: string) {
86548653
}
86558654
`);
86568655

8657-
env.driveMain();
8656+
env.driveMain();
86588657

8659-
const jsContents = env.getContents('test.js');
8660-
const dtsContents = env.getContents('test.d.ts');
8658+
const jsContents = env.getContents('test.js');
8659+
const dtsContents = env.getContents('test.d.ts');
86618660

8662-
expect(jsContents)
8663-
.toContain(
8664-
'inputs: { value: [i0.ɵɵInputFlags.HasTransform, "value", "value", toNumber] }');
8665-
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
8666-
expect(dtsContents).toContain('import * as i1 from "./types"');
8667-
expect(dtsContents)
8668-
.toContain(
8669-
'static ngAcceptInputType_value: boolean | string | i1.GenericWrapper<string>;');
8670-
});
8661+
expect(jsContents)
8662+
.toContain(
8663+
'inputs: { value: [i0.ɵɵInputFlags.HasDecoratorInputTransform, "value", "value", toNumber] }');
8664+
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
8665+
expect(dtsContents).toContain('import * as i1 from "./types"');
8666+
expect(dtsContents)
8667+
.toContain(
8668+
'static ngAcceptInputType_value: boolean | string | i1.GenericWrapper<string>;');
8669+
});
86718670

86728671
it('should compile an input with a transform function that contains nested generic parameters',
86738672
() => {
@@ -8702,7 +8701,7 @@ function allTests(os: string) {
87028701

87038702
expect(jsContents)
87048703
.toContain(
8705-
'inputs: { value: [i0.ɵɵInputFlags.HasTransform, "value", "value", toNumber] }');
8704+
'inputs: { value: [i0.ɵɵInputFlags.HasDecoratorInputTransform, "value", "value", toNumber] }');
87068705
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
87078706
expect(dtsContents).toContain('import * as i1 from "./types"');
87088707
expect(dtsContents).toContain('import * as i2 from "./other-types"');
@@ -8740,7 +8739,7 @@ function allTests(os: string) {
87408739
expect(jsContents).toContain(`import { externalToNumber } from 'external';`);
87418740
expect(jsContents)
87428741
.toContain(
8743-
'inputs: { value: [i0.ɵɵInputFlags.HasTransform, "value", "value", externalToNumber] }');
8742+
'inputs: { value: [i0.ɵɵInputFlags.HasDecoratorInputTransform, "value", "value", externalToNumber] }');
87448743
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
87458744
expect(dtsContents).toContain('import * as i1 from "external";');
87468745
expect(dtsContents).toContain('static ngAcceptInputType_value: i1.ExternalToNumberType;');
@@ -8772,7 +8771,7 @@ function allTests(os: string) {
87728771

87738772
expect(jsContents)
87748773
.toContain(
8775-
'inputs: { value: [i0.ɵɵInputFlags.HasTransform, "value", "value", (value) => value ? 1 : 0] }');
8774+
'inputs: { value: [i0.ɵɵInputFlags.HasDecoratorInputTransform, "value", "value", (value) => value ? 1 : 0] }');
87768775
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
87778776
expect(dtsContents).toContain('import * as i1 from "external";');
87788777
expect(dtsContents).toContain('static ngAcceptInputType_value: i1.ExternalToNumberType;');
@@ -8801,7 +8800,7 @@ function allTests(os: string) {
88018800

88028801
expect(jsContents)
88038802
.toContain(
8804-
'inputs: { value: [i0.ɵɵInputFlags.HasTransform, "value", "value", toBoolean] }');
8803+
'inputs: { value: [i0.ɵɵInputFlags.HasDecoratorInputTransform, "value", "value", toBoolean] }');
88058804
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
88068805
expect(dtsContents)
88078806
.toContain(`static ngAcceptInputType_value: boolean | "" | "true" | "false";`);
@@ -8826,7 +8825,7 @@ function allTests(os: string) {
88268825

88278826
expect(jsContents)
88288827
.toContain(
8829-
'inputs: { value: [i0.ɵɵInputFlags.HasTransform, "value", "value", toNumber] }');
8828+
'inputs: { value: [i0.ɵɵInputFlags.HasDecoratorInputTransform, "value", "value", toNumber] }');
88308829
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
88318830
expect(dtsContents).toContain('static ngAcceptInputType_value: boolean | string;');
88328831
});
@@ -8850,7 +8849,7 @@ function allTests(os: string) {
88508849

88518850
expect(jsContents)
88528851
.toContain(
8853-
'inputs: { value: [i0.ɵɵInputFlags.HasTransform, "value", "value", toNumber] }');
8852+
'inputs: { value: [i0.ɵɵInputFlags.HasDecoratorInputTransform, "value", "value", toNumber] }');
88548853
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
88558854
expect(dtsContents).toContain('static ngAcceptInputType_value: unknown;');
88568855
});
@@ -8877,7 +8876,7 @@ function allTests(os: string) {
88778876

88788877
expect(jsContents)
88798878
.toContain(
8880-
'inputs: { value: [i0.ɵɵInputFlags.HasTransform, "value", "value", toNumber] }');
8879+
'inputs: { value: [i0.ɵɵInputFlags.HasDecoratorInputTransform, "value", "value", toNumber] }');
88818880
expect(jsContents)
88828881
.toContain('features: [i0.ɵɵInputTransformsFeature, i0.ɵɵInheritDefinitionFeature]');
88838882
expect(dtsContents).toContain('static ngAcceptInputType_value: boolean | string;');
@@ -8907,7 +8906,7 @@ function allTests(os: string) {
89078906

89088907
expect(jsContents)
89098908
.toContain(
8910-
'inputs: { element: [i0.ɵɵInputFlags.HasTransform, "element", "element", coerceElement] }');
8909+
'inputs: { element: [i0.ɵɵInputFlags.HasDecoratorInputTransform, "element", "element", coerceElement] }');
89118910
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
89128911
expect(dtsContents)
89138912
.toContain(

‎packages/compiler/src/core.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ export interface Input {
4343
export enum InputFlags {
4444
None = 0,
4545
SignalBased = 1 << 0,
46-
HasTransform = 1 << 1,
46+
HasDecoratorInputTransform = 1 << 1,
4747
}
4848

4949
export interface Output {

‎packages/compiler/src/render3/partial/directive.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,9 @@ function getMinimumVersionForPartialOutput(meta: R3DirectiveMetadata): string {
114114
// Note: in order to allow consuming Angular libraries that have been compiled with 16.1+ in
115115
// Angular 16.0, we only force a minimum version of 16.1 if input transform feature as introduced
116116
// in 16.1 is actually used.
117-
const hasTransformFunctions =
117+
const hasDecoratorTransformFunctions =
118118
Object.values(meta.inputs).some(input => input.transformFunction !== null);
119-
if (hasTransformFunctions) {
119+
if (hasDecoratorTransformFunctions) {
120120
minVersion = '16.1.0';
121121
}
122122

‎packages/compiler/src/render3/view/util.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -207,27 +207,27 @@ export function conditionallyCreateDirectiveBindingLiteral(
207207
publicName = value.bindingPropertyName;
208208

209209
const differentDeclaringName = publicName !== declaredName;
210-
const hasTransform = value.transformFunction !== null;
210+
const hasDecoratorInputTransform = value.transformFunction !== null;
211211

212212
// Build up input flags
213213
let flags: o.Expression|null = null;
214214
if (value.isSignal) {
215215
flags = bitwiseAndInputFlagsExpr(InputFlags.SignalBased, flags);
216216
}
217-
if (value.transformFunction !== null) {
218-
flags = bitwiseAndInputFlagsExpr(InputFlags.HasTransform, flags);
217+
if (hasDecoratorInputTransform) {
218+
flags = bitwiseAndInputFlagsExpr(InputFlags.HasDecoratorInputTransform, flags);
219219
}
220220

221-
// Inputs, compared to outputs, will track their declared name (for `ngOnChanges`), or support
222-
// transform functions, or store flag information if there is any.
223-
if (forInputs && (differentDeclaringName || hasTransform || flags !== null)) {
221+
// Inputs, compared to outputs, will track their declared name (for `ngOnChanges`), support
222+
// decorator input transform functions, or store flag information if there is any.
223+
if (forInputs && (differentDeclaringName || hasDecoratorInputTransform || flags !== null)) {
224224
const flagsExpr = flags ?? o.importExpr(R3.InputFlags).prop(InputFlags[InputFlags.None]);
225225
const result: o.Expression[] = [flagsExpr, asLiteral(publicName)];
226226

227-
if (differentDeclaringName || hasTransform) {
227+
if (differentDeclaringName || hasDecoratorInputTransform) {
228228
result.push(asLiteral(declaredName));
229229

230-
if (hasTransform) {
230+
if (hasDecoratorInputTransform) {
231231
result.push(value.transformFunction!);
232232
}
233233
}

‎packages/core/src/render3/instructions/write_to_directive_input.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,14 @@ export function writeToDirectiveInput<T>(
2828
inputSignalNode = field[SIGNAL];
2929
}
3030

31-
if ((flags & InputFlags.HasTransform) !== 0) {
32-
if (inputSignalNode !== null) {
33-
value = inputSignalNode.transformFn!(value);
34-
} else {
35-
value = def.inputTransforms![privateName]!.call(instance, value);
36-
}
31+
// If there is a signal node and a transform, run it before potentially
32+
// delegating to features like `NgOnChanges`.
33+
if (inputSignalNode !== null && inputSignalNode.transformFn !== undefined) {
34+
value = inputSignalNode.transformFn(value);
35+
}
36+
// If there is a decorator input transform, run it.
37+
if ((flags & InputFlags.HasDecoratorInputTransform) !== 0) {
38+
value = def.inputTransforms![privateName]!.call(instance, value);
3739
}
3840

3941
if (def.setInput !== null) {

‎packages/core/src/render3/interfaces/definition.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ export interface PipeType<T> extends Type<T> {
9292
export enum InputFlags {
9393
None = 0,
9494
SignalBased = 1 << 0,
95-
HasTransform = 1 << 1,
95+
HasDecoratorInputTransform = 1 << 1,
9696
}
9797

9898
/**

‎packages/core/test/render3/jit/declare_component_spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ describe('component declaration jit compilation', () => {
7575

7676
expectComponentDef(def, {
7777
inputs: {
78-
'bindingName': ['minifiedClassProperty', InputFlags.HasTransform],
78+
'bindingName': ['minifiedClassProperty', InputFlags.HasDecoratorInputTransform],
7979
},
8080
inputTransforms: {
8181
'minifiedClassProperty': transformFn,

0 commit comments

Comments
 (0)