Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): add ability to transform input values #50420

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions goldens/public-api/core/index.md
Expand Up @@ -121,6 +121,9 @@ export interface AttributeDecorator {
new (name: string): Attribute;
}

// @public
export function booleanAttribute(value: unknown): boolean;

// @public
export interface BootstrapOptions {
ngZone?: NgZone | 'zone.js' | 'noop';
Expand Down Expand Up @@ -481,6 +484,7 @@ export interface Directive {
name: string;
alias?: string;
required?: boolean;
transform?: (value: any) => any;
} | string)[];
jit?: true;
outputs?: string[];
Expand Down Expand Up @@ -822,6 +826,7 @@ export interface InjectorType<T> extends Type<T> {
export interface Input {
alias?: string;
required?: boolean;
transform?: (value: any) => any;
}

// @public (undocumented)
Expand Down Expand Up @@ -1059,6 +1064,9 @@ export interface NgZoneOptions {
// @public
export const NO_ERRORS_SCHEMA: SchemaMetadata;

// @public
export function numberAttribute(value: unknown, fallbackValue?: number): number;

// @public
export interface OnChanges {
ngOnChanges(changes: SimpleChanges): void;
Expand Down
28 changes: 27 additions & 1 deletion packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
Expand Up @@ -8565,7 +8565,7 @@ function allTests(os: string) {

expect(jsContents).toContain('inputs: { value: ["value", "value", toNumber] }');
expect(jsContents)
.toContain('features: [i0.ɵɵStandaloneFeature, i0.ɵɵInputTransformsFeature]');
.toContain('features: [i0.ɵɵInputTransformsFeature, i0.ɵɵStandaloneFeature]');
expect(dtsContents).toContain('static ngAcceptInputType_value: boolean | string;');
});

Expand Down Expand Up @@ -8748,6 +8748,32 @@ function allTests(os: string) {
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
expect(dtsContents).toContain('static ngAcceptInputType_value: unknown;');
});

it('should insert the InputTransformsFeature before the InheritDefinitionFeature', () => {
env.write('/test.ts', `
import {Directive, Input} from '@angular/core';

function toNumber(value: boolean | string) { return 1; }

@Directive()
export class ParentDir {}

@Directive()
export class Dir extends ParentDir {
@Input({transform: toNumber}) value!: number;
}
`);

env.driveMain();

const jsContents = env.getContents('test.js');
const dtsContents = env.getContents('test.d.ts');

expect(jsContents).toContain('inputs: { value: ["value", "value", toNumber] }');
expect(jsContents)
.toContain('features: [i0.ɵɵInputTransformsFeature, i0.ɵɵInheritDefinitionFeature]');
expect(dtsContents).toContain('static ngAcceptInputType_value: boolean | string;');
});
});
});

Expand Down
25 changes: 13 additions & 12 deletions packages/compiler/src/jit_compiler_facade.ts
Expand Up @@ -325,8 +325,7 @@ function convertDirectiveFacadeToMetadata(facade: R3DirectiveMetadataFacade): R3
bindingPropertyName: ann.alias || field,
classPropertyName: field,
required: ann.required || false,
// TODO(crisbeto): resolve transform function reference here.
transformFunction: null,
transformFunction: ann.transform != null ? new WrappedNodeExpr(ann.transform) : null,
};
} else if (isOutput(ann)) {
outputsFromType[field] = ann.alias || field;
Expand Down Expand Up @@ -648,45 +647,47 @@ function isOutput(value: any): value is Output {
return value.ngMetadataName === 'Output';
}

function inputsMappingToInputMetadata(
inputs: Record<string, string|[string, string, InputTransformFunction?]>) {
function inputsMappingToInputMetadata(inputs: Record<string, string|[string, string, InputTransformFunction?]>) {
return Object.keys(inputs).reduce<InputMap>((result, key) => {
const value = inputs[key];

// TODO(crisbeto): resolve transform function reference here.
if (typeof value === 'string') {
result[key] = {
bindingPropertyName: value,
classPropertyName: value,
transformFunction: null,
required: false,
transformFunction: null
};
} else {
result[key] = {
bindingPropertyName: value[0],
classPropertyName: value[1],
transformFunction: value[2] || null,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't you keep these properties sorted because of *morphism? (or at least in the same order as other usages)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if that makes a difference. My understanding is that only the shape of the object matters (e.g. it has the same set of properties).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually the order in which you add the properties is relevant and that also applies to object literals

required: false,
transformFunction: null
};
}

return result;
}, {});
}

function parseInputsArray(values: (string|{name: string, alias?: string, required?: boolean})[]) {
function parseInputsArray(
values: (string|{name: string, alias?: string, required?: boolean, transform?: Function})[]) {
return values.reduce<InputMap>((results, value) => {
// TODO(crisbeto): resolve transform function reference here.
if (typeof value === 'string') {
const [bindingPropertyName, classPropertyName] = parseMappingString(value);
results[classPropertyName] =
{bindingPropertyName, classPropertyName, required: false, transformFunction: null};
results[classPropertyName] = {
bindingPropertyName,
classPropertyName,
required: false,
transformFunction: null,
};
} else {
results[value.name] = {
bindingPropertyName: value.alias || value.name,
classPropertyName: value.name,
required: value.required || false,
transformFunction: null
transformFunction: value.transform != null ? new WrappedNodeExpr(value.transform) : null,
};
}
return results;
Expand Down
13 changes: 6 additions & 7 deletions packages/compiler/src/render3/view/compiler.ts
Expand Up @@ -111,7 +111,12 @@ function addFeatures(
}
features.push(o.importExpr(R3.ProvidersFeature).callFn(args));
}

for (const key of inputKeys) {
if (meta.inputs[key].transformFunction !== null) {
features.push(o.importExpr(R3.InputTransformsFeatureFeature));
break;
}
}
if (meta.usesInheritance) {
features.push(o.importExpr(R3.InheritDefinitionFeature));
}
Expand All @@ -129,12 +134,6 @@ function addFeatures(
features.push(o.importExpr(R3.HostDirectivesFeature).callFn([createHostDirectivesFeatureArg(
meta.hostDirectives)]));
}
for (const key of inputKeys) {
if (meta.inputs[key].transformFunction !== null) {
features.push(o.importExpr(R3.InputTransformsFeatureFeature));
break;
}
}
if (features.length) {
definitionMap.set('features', o.literalArr(features));
}
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/core.ts
Expand Up @@ -42,6 +42,7 @@ export {createComponent, reflectComponentType, ComponentMirror} from './render3/
export {isStandalone} from './render3/definition';
export {ApplicationConfig, mergeApplicationConfig} from './application_config';
export {makeStateKey, StateKey, TransferState} from './transfer_state';
export {booleanAttribute, numberAttribute} from './util/coercion';

import {global} from './util/global';
if (typeof ngDevMode !== 'undefined' && ngDevMode) {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/core_private_export.ts
Expand Up @@ -30,7 +30,7 @@ export {_sanitizeHtml as ɵ_sanitizeHtml} from './sanitization/html_sanitizer';
export {_sanitizeUrl as ɵ_sanitizeUrl} from './sanitization/url_sanitizer';
export {setAlternateWeakRefImpl as ɵsetAlternateWeakRefImpl} from './signals';
export {TESTABILITY as ɵTESTABILITY, TESTABILITY_GETTER as ɵTESTABILITY_GETTER} from './testability/testability';
export {coerceToBoolean as ɵcoerceToBoolean} from './util/coercion';
export {booleanAttribute, numberAttribute} from './util/coercion';
export {devModeEqual as ɵdevModeEqual} from './util/comparison';
export {global as ɵglobal} from './util/global';
export {isPromise as ɵisPromise, isSubscribable as ɵisSubscribable} from './util/lang';
Expand Down
12 changes: 11 additions & 1 deletion packages/core/src/metadata/directives.ts
Expand Up @@ -182,7 +182,12 @@ export interface Directive {
* ```
*
*/
inputs?: ({name: string, alias?: string, required?: boolean}|string)[];
inputs?: ({
name: string,
alias?: string,
required?: boolean,
transform?: (value: any) => any,
}|string)[];

/**
* Enumerates the set of event-bound output properties.
Expand Down Expand Up @@ -817,6 +822,11 @@ export interface Input {
* Whether the input is required for the directive to function.
*/
required?: boolean;

/**
* Function with which to transform the input value before assigning it to the directive instance.
*/
transform?: (value: any) => any;
}

/**
Expand Down
27 changes: 21 additions & 6 deletions packages/core/src/render3/definition.ts
Expand Up @@ -18,7 +18,7 @@ import {initNgDevMode} from '../util/ng_dev_mode';
import {stringify} from '../util/stringify';

import {NG_COMP_DEF, NG_DIR_DEF, NG_MOD_DEF, NG_PIPE_DEF} from './fields';
import {ComponentDef, ComponentDefFeature, ComponentTemplate, ComponentType, ContentQueriesFunction, DependencyTypeList, DirectiveDef, DirectiveDefFeature, DirectiveDefListOrFactory, HostBindingsFunction, PipeDef, PipeDefListOrFactory, TypeOrFactory, ViewQueriesFunction} from './interfaces/definition';
import {ComponentDef, ComponentDefFeature, ComponentTemplate, ComponentType, ContentQueriesFunction, DependencyTypeList, DirectiveDef, DirectiveDefFeature, DirectiveDefListOrFactory, HostBindingsFunction, InputTransformFunction, PipeDef, PipeDefListOrFactory, TypeOrFactory, ViewQueriesFunction} from './interfaces/definition';
import {TAttributes, TConstantsOrFactory} from './interfaces/node';
import {CssSelectorList} from './interfaces/projection';
import {stringifyCSSSelectorList} from './node_selector_matcher';
Expand All @@ -35,7 +35,7 @@ interface DirectiveDefinition<T> {
/**
* A map of input names.
*
* The format is in: `{[actualPropertyName: string]:(string|[string, string])}`.
* The format is in: `{[actualPropertyName: string]:(string|[string, string, Function])}`.
*
* Given:
* ```
Expand All @@ -45,6 +45,9 @@ interface DirectiveDefinition<T> {
*
* @Input('publicInput2')
* declaredInput2: string;
*
* @Input({transform: (value: boolean) => value ? 1 : 0})
* transformedInput3: number;
* }
* ```
*
Expand All @@ -53,6 +56,11 @@ interface DirectiveDefinition<T> {
* {
* publicInput1: 'publicInput1',
* declaredInput2: ['declaredInput2', 'publicInput2'],
* transformedInput3: [
* 'transformedInput3',
* 'transformedInput3',
* (value: boolean) => value ? 1 : 0
* ]
* }
* ```
*
Expand All @@ -61,6 +69,11 @@ interface DirectiveDefinition<T> {
* {
* minifiedPublicInput1: 'publicInput1',
* minifiedDeclaredInput2: [ 'publicInput2', 'declaredInput2'],
* minifiedTransformedInput3: [
* 'transformedInput3',
* 'transformedInput3',
* (value: boolean) => value ? 1 : 0
* ]
* }
* ```
*
Expand All @@ -75,7 +88,7 @@ interface DirectiveDefinition<T> {
* this reason `NgOnChanges` will be deprecated and removed in future version and this
* API will be simplified to be consistent with `output`.
*/
inputs?: {[P in keyof T]?: string|[string, string]};
inputs?: {[P in keyof T]?: string|[string, string, InputTransformFunction?]};

/**
* A map of output names.
Expand Down Expand Up @@ -484,13 +497,13 @@ export function ɵɵsetNgModuleScope(type: any, scope: {

*/
function invertObject<T>(
obj?: {[P in keyof T]?: string|[string, string]},
secondary?: {[key: string]: string}): {[P in keyof T]: string} {
obj?: {[P in keyof T]?: string|[string, string, ...unknown[]]},
secondary?: Record<string, string>): {[P in keyof T]: string} {
if (obj == null) return EMPTY_OBJ as any;
const newLookup: any = {};
for (const minifiedKey in obj) {
if (obj.hasOwnProperty(minifiedKey)) {
let publicName: string|[string, string] = obj[minifiedKey]!;
let publicName: string|[string, string, ...unknown[]] = obj[minifiedKey]!;
let declaredName = publicName;
if (Array.isArray(publicName)) {
declaredName = publicName[1];
Expand Down Expand Up @@ -626,6 +639,8 @@ function getNgDirectiveDef<T>(directiveDefinition: DirectiveDefinition<T>):
hostAttrs: directiveDefinition.hostAttrs || null,
contentQueries: directiveDefinition.contentQueries || null,
declaredInputs,
inputTransforms: null,
inputConfig: directiveDefinition.inputs || EMPTY_OBJ,
exportAs: directiveDefinition.exportAs || null,
standalone: directiveDefinition.standalone === true,
signals: directiveDefinition.signals === true,
Expand Down
Expand Up @@ -59,6 +59,7 @@ export function ɵɵInheritDefinitionFeature(definition: DirectiveDef<any>|Compo
// would've justified object creation. Unwrap them if necessary.
const writeableDef = definition as WritableDef;
writeableDef.inputs = maybeUnwrapEmpty(definition.inputs);
writeableDef.inputTransforms = maybeUnwrapEmpty(definition.inputTransforms);
writeableDef.declaredInputs = maybeUnwrapEmpty(definition.declaredInputs);
writeableDef.outputs = maybeUnwrapEmpty(definition.outputs);

Expand All @@ -77,6 +78,13 @@ export function ɵɵInheritDefinitionFeature(definition: DirectiveDef<any>|Compo
fillProperties(definition.declaredInputs, superDef.declaredInputs);
fillProperties(definition.outputs, superDef.outputs);

if (superDef.inputTransforms !== null) {
if (writeableDef.inputTransforms === null) {
writeableDef.inputTransforms = {};
}
fillProperties(writeableDef.inputTransforms, superDef.inputTransforms);
}

// Merge animations metadata.
// If `superDef` is a Component, the `data` field is present (defaults to an empty object).
if (isComponentDef(superDef) && superDef.data.animation) {
Expand Down
28 changes: 25 additions & 3 deletions packages/core/src/render3/features/input_transforms_feature.ts
Expand Up @@ -6,10 +6,32 @@
* found in the LICENSE file at https://angular.io/license
*/

import {ComponentDef, DirectiveDef} from '../interfaces/definition';
import {Mutable} from '../../interface/type';
import {DirectiveDef, InputTransformFunction} from '../interfaces/definition';

// TODO(crisbeto): move input transforms runtime functionality here.
/**
* Decorates the directive definition with support for input transform functions.
*
* If the directive uses inheritance, the feature should be included before the
* `InheritDefinitionFeature` to ensure that the `inputTransforms` field is populated.
*
* @codeGenApi
*/
export function ɵɵInputTransformsFeature(definition: DirectiveDef<any>|ComponentDef<any>): void {}
export function ɵɵInputTransformsFeature(definition: DirectiveDef<unknown>): void {
const inputs = definition.inputConfig;
const inputTransforms: Record<string, InputTransformFunction> = {};

for (const minifiedKey in inputs) {
if (inputs.hasOwnProperty(minifiedKey)) {
// Note: the private names are used for the keys, rather than the public ones, because public
// names can be re-aliased in host directives which would invalidate the lookup.
const value = inputs[minifiedKey];
if (Array.isArray(value) && value[2]) {
inputTransforms[minifiedKey] = value[2];
}
}
}

(definition as Mutable<DirectiveDef<unknown>, 'inputTransforms'>).inputTransforms =
inputTransforms;
}
4 changes: 4 additions & 0 deletions packages/core/src/render3/instructions/shared.ts
Expand Up @@ -1311,6 +1311,10 @@ function writeToDirectiveInput<T>(
def: DirectiveDef<T>, instance: T, publicName: string, privateName: string, value: string) {
const prevConsumer = setActiveConsumer(null);
try {
const inputTransforms = def.inputTransforms;
if (inputTransforms !== null && inputTransforms.hasOwnProperty(privateName)) {
value = inputTransforms[privateName].call(instance, value);
}
if (def.setInput !== null) {
def.setInput(instance, value, publicName, privateName);
} else {
Expand Down