Skip to content

Commit

Permalink
refactor(compiler-cli): reference InputFlags enum directly for full…
Browse files Browse the repository at this point in the history
… compiler output (angular#53571)

Instead of computing the bit input flags at compile-time and inling
the final bit flag number, we will use the `InputFlags` enum directly.
This is a little more code in the compiler side, but will allow us to
have better debuggable development code, and also prevents problems
where runtime flag bitmasks differ from the compiler flag bitmasks.

This is in practice a noop for optimized applications as the enum values
would be inlined anyway. This matches existing compiler emit for e.g.
change detection strategy, or view encapsulation enums.

PR Close angular#53571
  • Loading branch information
devversion authored and amilamen committed Jan 26, 2024
1 parent 30d864c commit 14abf76
Show file tree
Hide file tree
Showing 13 changed files with 93 additions and 36 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
LifecycleComp.ɵcmp = /*@__PURE__*/ $r3$.ɵɵdefineComponent({
type: LifecycleComp,
selectors: [["lifecycle-comp"]],
inputs: {nameMin: [0, "name", "nameMin"]},
inputs: {nameMin: [$r3$.ɵɵInputFlags.None, "name", "nameMin"]},
features: [$r3$.ɵɵNgOnChangesFeature],
decls: 0,
vars: 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ MyComponent.ɵcmp = /*@__PURE__*/ $r3$.ɵɵdefineComponent({
inputs:{
componentInput: "componentInput",
originalComponentInput: [0, "renamedComponentInput", "originalComponentInput"]
originalComponentInput: [$r3$.ɵɵInputFlags.None, "renamedComponentInput", "originalComponentInput"]
},
outputs: {
componentOutput: "componentOutput",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ MyDirective.ɵdir = /*@__PURE__*/ $r3$.ɵɵdefineDirective({
inputs:{
directiveInput: "directiveInput",
originalDirectiveInput: [0, "renamedDirectiveInput", "originalDirectiveInput"]
originalDirectiveInput: [$r3$.ɵɵInputFlags.None, "renamedDirectiveInput", "originalDirectiveInput"]
},
outputs: {
directiveOutput: "directiveOutput",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
MyDirective.ɵdir = /*@__PURE__*/ $r3$.ɵɵdefineDirective({
inputs: {
functionDeclarationInput: [2, "functionDeclarationInput", "functionDeclarationInput", toNumber],
inlineFunctionInput: [2, "inlineFunctionInput", "inlineFunctionInput", (value, _) => value ? 1 : 0]
functionDeclarationInput: [$r3$.ɵɵInputFlags.HasTransform, "functionDeclarationInput", "functionDeclarationInput", toNumber],
inlineFunctionInput: [$r3$.ɵɵInputFlags.HasTransform, "inlineFunctionInput", "inlineFunctionInput", (value, _) => value ? 1 : 0]
},
features: [$r3$.ɵɵInputTransformsFeature]
});
10 changes: 6 additions & 4 deletions packages/compiler-cli/test/ngtsc/authoring_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ runInEachFileSystem(() => {
`);
env.driveMain();
const js = env.getContents('test.js');
expect(js).toContain('inputs: { data: [1, "data"] }');
expect(js).toContain('inputs: { data: [i0.ɵɵInputFlags.SignalBased, "data"] }');
});

it('should handle an alias configured, primitive valued input', () => {
Expand All @@ -49,7 +49,7 @@ runInEachFileSystem(() => {
`);
env.driveMain();
const js = env.getContents('test.js');
expect(js).toContain('inputs: { data: [1, "publicName", "data"] }');
expect(js).toContain('inputs: { data: [i0.ɵɵInputFlags.SignalBased, "publicName", "data"] }');
});

it('should error if a required input declares an initial value', () => {
Expand Down Expand Up @@ -82,7 +82,8 @@ runInEachFileSystem(() => {
}
`);
env.driveMain();
expect(env.getContents('test.js')).toContain(`inputs: { data: [1, "data"] }`);
expect(env.getContents('test.js'))
.toContain(`inputs: { data: [i0.ɵɵInputFlags.SignalBased, "data"] }`);
expect(env.getContents('test.d.ts')).toContain('"required": true; "isSignal": true;');
expect(env.getContents('test.d.ts'))
.withContext(
Expand All @@ -101,7 +102,8 @@ runInEachFileSystem(() => {
}
`);
env.driveMain();
expect(env.getContents('test.js')).toContain(`inputs: { data: [1, "data"] }`);
expect(env.getContents('test.js'))
.toContain(`inputs: { data: [i0.ɵɵInputFlags.SignalBased, "data"] }`);
});

describe('type checking', () => {
Expand Down
18 changes: 13 additions & 5 deletions packages/compiler-cli/test/ngtsc/local_compilation_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1052,7 +1052,8 @@ runInEachFileSystem(() => {
env.driveMain();
const jsContents = env.getContents('test.js');

expect(jsContents).toContain('inputs: { x: [2, "x", "x", externalFunc] }');
expect(jsContents)
.toContain('inputs: { x: [i0.ɵɵInputFlags.HasTransform, "x", "x", externalFunc] }');
});

it('should generate input info for transform function imported externally using namespace',
Expand All @@ -1073,7 +1074,9 @@ runInEachFileSystem(() => {
env.driveMain();
const jsContents = env.getContents('test.js');

expect(jsContents).toContain('inputs: { x: [2, "x", "x", n.externalFunc] }');
expect(jsContents)
.toContain(
'inputs: { x: [i0.ɵɵInputFlags.HasTransform, "x", "x", n.externalFunc] }');
});

it('should generate input info for transform function defined locally', () => {
Expand All @@ -1096,7 +1099,8 @@ runInEachFileSystem(() => {
env.driveMain();
const jsContents = env.getContents('test.js');

expect(jsContents).toContain('inputs: { x: [2, "x", "x", localFunc] }');
expect(jsContents)
.toContain('inputs: { x: [i0.ɵɵInputFlags.HasTransform, "x", "x", localFunc] }');
});

it('should generate input info for inline transform function', () => {
Expand All @@ -1115,7 +1119,9 @@ runInEachFileSystem(() => {
env.driveMain();
const jsContents = env.getContents('test.js');

expect(jsContents).toContain('inputs: { x: [2, "x", "x", (v) => v + \'TRANSFORMED!\'] }');
expect(jsContents)
.toContain(
'inputs: { x: [i0.ɵɵInputFlags.HasTransform, "x", "x", (v) => v + \'TRANSFORMED!\'] }');
});

it('should not check inline function param type', () => {
Expand All @@ -1134,7 +1140,9 @@ runInEachFileSystem(() => {
env.driveMain();
const jsContents = env.getContents('test.js');

expect(jsContents).toContain('inputs: { x: [2, "x", "x", v => v + \'TRANSFORMED!\'] }');
expect(jsContents)
.toContain(
'inputs: { x: [i0.ɵɵInputFlags.HasTransform, "x", "x", v => v + \'TRANSFORMED!\'] }');
});
});
});
Expand Down
46 changes: 33 additions & 13 deletions packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5800,8 +5800,8 @@ function allTests(os: string) {
"track-name": "track-name",
inputTrackName: "inputTrackName",
"src.xl": "src.xl",
trackType: [0, "track-type", "trackType"],
trackName: [0, "track-name", "trackName"]
trackType: [i0.ɵɵInputFlags.None, "track-type", "trackType"],
trackName: [i0.ɵɵInputFlags.None, "track-name", "trackName"]
},
outputs: {
"output-track-type": "output-track-type",
Expand Down Expand Up @@ -8603,7 +8603,9 @@ function allTests(os: string) {
const jsContents = env.getContents('test.js');
const dtsContents = env.getContents('test.d.ts');

expect(jsContents).toContain('inputs: { value: [2, "value", "value", toNumber] }');
expect(jsContents)
.toContain(
'inputs: { value: [i0.ɵɵInputFlags.HasTransform, "value", "value", toNumber] }');
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
expect(dtsContents).toContain('static ngAcceptInputType_value: boolean | string;');
});
Expand All @@ -8625,7 +8627,9 @@ function allTests(os: string) {
const jsContents = env.getContents('test.js');
const dtsContents = env.getContents('test.d.ts');

expect(jsContents).toContain('inputs: { value: [2, "value", "value", toNumber] }');
expect(jsContents)
.toContain(
'inputs: { value: [i0.ɵɵInputFlags.HasTransform, "value", "value", toNumber] }');
expect(jsContents)
.toContain('features: [i0.ɵɵInputTransformsFeature, i0.ɵɵStandaloneFeature]');
expect(dtsContents).toContain('static ngAcceptInputType_value: boolean | string;');
Expand Down Expand Up @@ -8655,7 +8659,9 @@ function allTests(os: string) {
const jsContents = env.getContents('test.js');
const dtsContents = env.getContents('test.d.ts');

expect(jsContents).toContain('inputs: { value: [2, "value", "value", toNumber] }');
expect(jsContents)
.toContain(
'inputs: { value: [i0.ɵɵInputFlags.HasTransform, "value", "value", toNumber] }');
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
expect(dtsContents).toContain('import * as i1 from "./types"');
expect(dtsContents)
Expand Down Expand Up @@ -8694,7 +8700,9 @@ function allTests(os: string) {
const jsContents = env.getContents('test.js');
const dtsContents = env.getContents('test.d.ts');

expect(jsContents).toContain('inputs: { value: [2, "value", "value", toNumber] }');
expect(jsContents)
.toContain(
'inputs: { value: [i0.ɵɵInputFlags.HasTransform, "value", "value", toNumber] }');
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
expect(dtsContents).toContain('import * as i1 from "./types"');
expect(dtsContents).toContain('import * as i2 from "./other-types"');
Expand Down Expand Up @@ -8730,7 +8738,9 @@ function allTests(os: string) {
const dtsContents = env.getContents('test.d.ts');

expect(jsContents).toContain(`import { externalToNumber } from 'external';`);
expect(jsContents).toContain('inputs: { value: [2, "value", "value", externalToNumber] }');
expect(jsContents)
.toContain(
'inputs: { value: [i0.ɵɵInputFlags.HasTransform, "value", "value", externalToNumber] }');
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
expect(dtsContents).toContain('import * as i1 from "external";');
expect(dtsContents).toContain('static ngAcceptInputType_value: i1.ExternalToNumberType;');
Expand Down Expand Up @@ -8761,7 +8771,8 @@ function allTests(os: string) {
const dtsContents = env.getContents('test.d.ts');

expect(jsContents)
.toContain('inputs: { value: [2, "value", "value", (value) => value ? 1 : 0] }');
.toContain(
'inputs: { value: [i0.ɵɵInputFlags.HasTransform, "value", "value", (value) => value ? 1 : 0] }');
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
expect(dtsContents).toContain('import * as i1 from "external";');
expect(dtsContents).toContain('static ngAcceptInputType_value: i1.ExternalToNumberType;');
Expand All @@ -8788,7 +8799,9 @@ function allTests(os: string) {
const jsContents = env.getContents('test.js');
const dtsContents = env.getContents('test.d.ts');

expect(jsContents).toContain('inputs: { value: [2, "value", "value", toBoolean] }');
expect(jsContents)
.toContain(
'inputs: { value: [i0.ɵɵInputFlags.HasTransform, "value", "value", toBoolean] }');
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
expect(dtsContents)
.toContain(`static ngAcceptInputType_value: boolean | "" | "true" | "false";`);
Expand All @@ -8811,7 +8824,9 @@ function allTests(os: string) {
const jsContents = env.getContents('test.js');
const dtsContents = env.getContents('test.d.ts');

expect(jsContents).toContain('inputs: { value: [2, "value", "value", toNumber] }');
expect(jsContents)
.toContain(
'inputs: { value: [i0.ɵɵInputFlags.HasTransform, "value", "value", toNumber] }');
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
expect(dtsContents).toContain('static ngAcceptInputType_value: boolean | string;');
});
Expand All @@ -8833,7 +8848,9 @@ function allTests(os: string) {
const jsContents = env.getContents('test.js');
const dtsContents = env.getContents('test.d.ts');

expect(jsContents).toContain('inputs: { value: [2, "value", "value", toNumber] }');
expect(jsContents)
.toContain(
'inputs: { value: [i0.ɵɵInputFlags.HasTransform, "value", "value", toNumber] }');
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
expect(dtsContents).toContain('static ngAcceptInputType_value: unknown;');
});
Expand All @@ -8858,7 +8875,9 @@ function allTests(os: string) {
const jsContents = env.getContents('test.js');
const dtsContents = env.getContents('test.d.ts');

expect(jsContents).toContain('inputs: { value: [2, "value", "value", toNumber] }');
expect(jsContents)
.toContain(
'inputs: { value: [i0.ɵɵInputFlags.HasTransform, "value", "value", toNumber] }');
expect(jsContents)
.toContain('features: [i0.ɵɵInputTransformsFeature, i0.ɵɵInheritDefinitionFeature]');
expect(dtsContents).toContain('static ngAcceptInputType_value: boolean | string;');
Expand Down Expand Up @@ -8887,7 +8906,8 @@ function allTests(os: string) {
const dtsContents = env.getContents('test.d.ts');

expect(jsContents)
.toContain('inputs: { element: [2, "element", "element", coerceElement] }');
.toContain(
'inputs: { element: [i0.ɵɵInputFlags.HasTransform, "element", "element", coerceElement] }');
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
expect(dtsContents)
.toContain(
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler/src/compiler_facade_interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export interface CompilerFacade {
}

export interface CoreEnvironment {
[name: string]: Function;
[name: string]: unknown;
}

export type ResourceLoader = {
Expand Down
5 changes: 5 additions & 0 deletions packages/compiler/src/render3/r3_identifiers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,11 @@ export class Identifiers {
moduleName: CORE,
};

static InputFlags: o.ExternalReference = {
name: 'ɵɵInputFlags',
moduleName: CORE,
};

// sanitization-related functions
static sanitizeHtml: o.ExternalReference = {name: 'ɵɵsanitizeHtml', moduleName: CORE};
static sanitizeStyle: o.ExternalReference = {name: 'ɵɵsanitizeStyle', moduleName: CORE};
Expand Down
30 changes: 24 additions & 6 deletions packages/compiler/src/render3/view/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,18 +210,19 @@ export function conditionallyCreateDirectiveBindingLiteral(
const hasTransform = value.transformFunction !== null;

// Build up input flags
let flags = 0;
let flags: o.Expression|null = null;
if (value.isSignal) {
flags = flags | InputFlags.SignalBased;
flags = bitwiseAndInputFlagsExpr(InputFlags.SignalBased, flags);
}
if (hasTransform) {
flags = flags | InputFlags.HasTransform;
if (value.transformFunction !== null) {
flags = bitwiseAndInputFlagsExpr(InputFlags.HasTransform, flags);
}

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

if (differentDeclaringName || hasTransform) {
result.push(asLiteral(declaredName));
Expand All @@ -246,6 +247,23 @@ export function conditionallyCreateDirectiveBindingLiteral(
}));
}

/** Gets an output AST expression referencing the given flag. */
function getInputFlagExpr(flag: InputFlags): o.Expression {
return o.importExpr(R3.InputFlags).prop(InputFlags[flag]);
}

/** Combines a given input flag with an existing flag expression, if present. */
function bitwiseAndInputFlagsExpr(flag: InputFlags, expr: o.Expression|null): o.Expression {
if (expr === null) {
return getInputFlagExpr(flag);
}
return new o.BinaryOperatorExpr(
o.BinaryOperator.BitwiseAnd,
expr,
getInputFlagExpr(flag),
);
}

/**
* Remove trailing null nodes as they are implied.
*/
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/compiler/compiler_facade_interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export interface CompilerFacade {
}

export interface CoreEnvironment {
[name: string]: Function;
[name: string]: unknown;
}

export type ResourceLoader = {
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/core_render3_private_export.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,5 +316,6 @@ export { AfterRenderEventManager as ɵAfterRenderEventManager, internalAfterNext
export {depsTracker as ɵdepsTracker, USE_RUNTIME_DEPS_TRACKER_FOR_JIT as ɵUSE_RUNTIME_DEPS_TRACKER_FOR_JIT} from './render3/deps_tracker/deps_tracker';
export {generateStandaloneInDeclarationsError as ɵgenerateStandaloneInDeclarationsError} from './render3/jit/module';
export {getAsyncClassMetadataFn as ɵgetAsyncClassMetadataFn} from './render3/metadata';
export {InputFlags as ɵɵInputFlags} from './render3/interfaces/definition';

// clang-format on
5 changes: 4 additions & 1 deletion packages/core/src/render3/jit/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {forwardRef, resolveForwardRef} from '../../di/forward_ref';
import {ɵɵinject, ɵɵinvalidFactoryDep} from '../../di/injector_compatibility';
import {ɵɵdefineInjectable, ɵɵdefineInjector} from '../../di/interface/defs';
import {registerNgModuleType} from '../../linker/ng_module_registration';
import {InputFlags} from '../../render3/interfaces/definition';
import * as iframe_attrs_validation from '../../sanitization/iframe_attrs_validation';
import * as sanitization from '../../sanitization/sanitization';
import * as r3 from '../index';
Expand All @@ -20,7 +21,7 @@ import * as r3 from '../index';
*
* This should be kept up to date with the public exports of @angular/core.
*/
export const angularCoreEnv: {[name: string]: Function} =
export const angularCoreEnv: {[name: string]: unknown} =
(() => ({
'ɵɵattribute': r3.ɵɵattribute,
'ɵɵattributeInterpolate1': r3.ɵɵattributeInterpolate1,
Expand Down Expand Up @@ -199,4 +200,6 @@ export const angularCoreEnv: {[name: string]: Function} =

'forwardRef': forwardRef,
'resolveForwardRef': resolveForwardRef,

'ɵɵInputFlags': InputFlags,
}))();

0 comments on commit 14abf76

Please sign in to comment.