From 3bd2fcd0440db57166cf39178d770c3e2265d3b4 Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Tue, 27 Jun 2023 11:36:42 +0200 Subject: [PATCH] Revert "feat(core): propertyInterpolationCreate" This reverts commit 2e3fe0747990647c5cfbba5429e90ed621b18e6e. Based on the recent design discussion we are going to express property interpolation as a computed function and reuse the propertyCreate infrastructure. --- .../render3/instructions/property_create.ts | 118 ++++++++++++++-- .../instructions/property_create_shared.ts | 121 ---------------- .../property_iterpolation_create.ts | 131 ------------------ .../input_interpolation.spec.ts | 2 +- 4 files changed, 111 insertions(+), 261 deletions(-) delete mode 100644 packages/core/src/render3/instructions/property_create_shared.ts delete mode 100644 packages/core/src/render3/instructions/property_iterpolation_create.ts diff --git a/packages/core/src/render3/instructions/property_create.ts b/packages/core/src/render3/instructions/property_create.ts index 63543c588bc..81567c6440e 100644 --- a/packages/core/src/render3/instructions/property_create.ts +++ b/packages/core/src/render3/instructions/property_create.ts @@ -7,14 +7,25 @@ */ -import {assertDefined} from '../../util/assert'; -import {EMPTY_ARRAY} from '../../util/empty'; -import {SanitizerFn} from '../interfaces/sanitization'; -import {HEADER_OFFSET} from '../interfaces/view'; -import {computed} from '../reactivity/computed'; -import {getCurrentTNode, getLView, getTView} from '../state'; +import {SIGNAL} from '@angular/core/primitives/signals'; -import {determineInputTargets, propertyUpdateDom, propertyUpdateInput} from './property_create_shared'; +import {validateAgainstEventProperties} from '../../sanitization/sanitization'; +import {assertDefined, assertIndexInRange} from '../../util/assert'; +import {EMPTY_ARRAY} from '../../util/empty'; +import {bindingUpdated} from '../bindings'; +import {DirectiveDef} from '../interfaces/definition'; +import {PropertyAliasValue, TNode} from '../interfaces/node'; +import {RComment, RElement} from '../interfaces/renderer_dom'; +import {SanitizerFn} from '../interfaces/sanitization'; +import {isComponentHost} from '../interfaces/type_checks'; +import {HEADER_OFFSET, RENDERER} from '../interfaces/view'; +import {computed} from '../reactivity/computed'; +import {InputSignalNode} from '../reactivity/input_signal'; +import {getCurrentTNode, getLView, getSelectedTNode, getTView, nextBindingIndex} from '../state'; +import {getNativeByTNode} from '../util/view_utils'; + +import {handleUnknownPropertyError, isPropertyValid} from './element_validation'; +import {mapPropName, markDirtyIfOnPush, setNgReflectProperties, writeToDirectiveInput} from './shared'; /** * TODO @@ -32,7 +43,32 @@ export function ɵɵpropertyCreate( assertDefined(tNode, `propertyCreate() must follow an actual element`); const inputData = tNode.inputs?.[propName] ?? EMPTY_ARRAY; - const {zoneTargets, signalInputs} = determineInputTargets(inputData, tView, lView); + + let signalInputs: InputSignalNode[]|null = null; + + // PERF: the fact that we need to iterate over all the inputs here isn't great. + // We might consider storing more info on TView + let zoneTargets: PropertyAliasValue|null = null; + for (let i = 0; i < inputData.length;) { + const directiveIndex = inputData[i++] as number; + const privateName = inputData[i++] as string; + const def = tView.data[directiveIndex] as DirectiveDef; + if (!def.signals) { + // TODO(pk): refactor - code flow with all those firstCreatePass checks becomes hard to follow + if (tView.firstCreatePass) { + (zoneTargets ??= []).push(directiveIndex, privateName); + } + } else { + ngDevMode && assertIndexInRange(lView, directiveIndex); + // PERF: megamorphic read on [privateName] access + const inputSignal = + lView[directiveIndex][privateName][SIGNAL] as InputSignalNode; + (signalInputs ??= []).push(inputSignal); + } + } + + zoneTargets ??= EMPTY_ARRAY; + signalInputs ??= EMPTY_ARRAY; // If there are multiple signal targets, or any zone targets, then wrap `expr` in a computed. This // ensures that the expression is only evaluated once, even if it has multiple consumers. Zone @@ -72,3 +108,69 @@ export function ɵɵpropertyCreate( return ɵɵpropertyCreate; } + +export function propertyUpdateDom( + nodeSlot: number, propName: string, expressionSlot: number, sanitizer: SanitizerFn|null): void { + const lView = getLView(); + const expr = lView[expressionSlot]; + + let value = expr(); + const bindingIndex = nextBindingIndex(); + if (!bindingUpdated(lView, bindingIndex, value)) { + return; + } + + const tView = getTView(); + const tNode = tView.data[nodeSlot] as TNode; + const element = getNativeByTNode(tNode, lView) as RElement | RComment; + + propName = mapPropName(propName); + + if (ngDevMode) { + validateAgainstEventProperties(propName); + if (!isPropertyValid(element, propName, tNode.value, tView.schemas)) { + handleUnknownPropertyError(propName, tNode.value, tNode.type, lView); + } + ngDevMode.rendererSetProperty++; + } + + // It is assumed that the sanitizer is only added when the compiler determines that the + // property is risky, so sanitization can be done without further checks. + value = sanitizer != null ? (sanitizer(value, tNode.value || '', propName) as any) : value; + lView[RENDERER].setProperty(element as RElement, propName, value); +} + +export function propertyUpdateInput( + propName: string, expressionSlot: number, targets: PropertyAliasValue): void { + const lView = getLView(); + const expr = lView[expressionSlot]; + const value = expr(); + + const tNode = getSelectedTNode(); + const tView = getTView(); + + ngDevMode && assertDefined(tNode.inputs, `Expected tNode to have inputs`); + + const bindingIndex = nextBindingIndex(); + if (!bindingUpdated(lView, bindingIndex, value)) { + return; + } + + for (let i = 0; i < targets.length;) { + const index = targets[i++] as number; + const privateName = targets[i++] as string; + ngDevMode && assertIndexInRange(lView, index); + const instance = lView[index]; + const def = tView.data[index] as DirectiveDef; + + writeToDirectiveInput(def, instance, propName, privateName, value); + } + + const element = getNativeByTNode(tNode, lView) as RElement | RComment; + if (isComponentHost(tNode)) { + markDirtyIfOnPush(lView, tNode.index); + } + if (ngDevMode) { + setNgReflectProperties(lView, element, tNode.type, targets, value); + } +} diff --git a/packages/core/src/render3/instructions/property_create_shared.ts b/packages/core/src/render3/instructions/property_create_shared.ts deleted file mode 100644 index ec50531cb9a..00000000000 --- a/packages/core/src/render3/instructions/property_create_shared.ts +++ /dev/null @@ -1,121 +0,0 @@ -/** - * @license - * Copyright Google LLC All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ - -import {validateAgainstEventProperties} from '../../sanitization/sanitization'; -import {SIGNAL} from '../../signals'; -import {assertDefined, assertIndexInRange} from '../../util/assert'; -import {EMPTY_ARRAY} from '../../util/empty'; -import {bindingUpdated} from '../bindings'; -import {DirectiveDef} from '../interfaces/definition'; -import {PropertyAliasValue, TNode} from '../interfaces/node'; -import {RComment, RElement} from '../interfaces/renderer_dom'; -import {SanitizerFn} from '../interfaces/sanitization'; -import {isComponentHost} from '../interfaces/type_checks'; -import {LView, RENDERER, TView} from '../interfaces/view'; -import {InputSignalNode} from '../reactivity/input_signal'; -import {getLView, getSelectedTNode, getTView, nextBindingIndex} from '../state'; -import {getNativeByTNode} from '../util/view_utils'; - -import {handleUnknownPropertyError, isPropertyValid} from './element_validation'; -import {mapPropName, markDirtyIfOnPush, setNgReflectProperties, writeToDirectiveInput} from './shared'; - -export function determineInputTargets(inputData: any[], tView: TView, lView: LView) { - let signalInputs: InputSignalNode[]|null = null; - let zoneTargets: PropertyAliasValue|null = null; - - // PERF: the fact that we need to iterate over all the inputs here isn't great. - // We might consider storing more info on TView - - for (let i = 0; i < inputData.length;) { - const directiveIndex = inputData[i++] as number; - const privateName = inputData[i++] as string; - const def = tView.data[directiveIndex] as DirectiveDef; - if (!def.signals) { - // TODO(pk): refactor - code flow with all those firstCreatePass checks becomes hard to follow - if (tView.firstCreatePass) { - (zoneTargets ??= []).push(directiveIndex, privateName); - } - } else { - ngDevMode && assertIndexInRange(lView, directiveIndex); - // PERF: megamorphic read on [privateName] access - const inputSignal = - lView[directiveIndex][privateName][SIGNAL] as InputSignalNode; - (signalInputs ??= []).push(inputSignal); - } - } - - zoneTargets ??= EMPTY_ARRAY; - signalInputs ??= EMPTY_ARRAY; - return {zoneTargets, signalInputs}; -} - -export function propertyUpdateDom( - nodeSlot: number, propName: string, expressionSlot: number, sanitizer: SanitizerFn|null): void { - const lView = getLView(); - const expr = lView[expressionSlot]; - - let value = expr(); - const bindingIndex = nextBindingIndex(); - if (!bindingUpdated(lView, bindingIndex, value)) { - return; - } - - const tView = getTView(); - const tNode = tView.data[nodeSlot] as TNode; - const element = getNativeByTNode(tNode, lView) as RElement | RComment; - - propName = mapPropName(propName); - - if (ngDevMode) { - validateAgainstEventProperties(propName); - if (!isPropertyValid(element, propName, tNode.value, tView.schemas)) { - handleUnknownPropertyError(propName, tNode.value, tNode.type, lView); - } - ngDevMode.rendererSetProperty++; - } - - // It is assumed that the sanitizer is only added when the compiler determines that the - // property is risky, so sanitization can be done without further checks. - value = sanitizer != null ? (sanitizer(value, tNode.value || '', propName) as any) : value; - lView[RENDERER].setProperty(element as RElement, propName, value); -} - -export function propertyUpdateInput( - propName: string, expressionSlot: number, targets: PropertyAliasValue): void { - const lView = getLView(); - const expr = lView[expressionSlot]; - const value = expr(); - - const tNode = getSelectedTNode(); - const tView = getTView(); - - ngDevMode && assertDefined(tNode.inputs, `Expected tNode to have inputs`); - - const bindingIndex = nextBindingIndex(); - if (!bindingUpdated(lView, bindingIndex, value)) { - return; - } - - for (let i = 0; i < targets.length;) { - const index = targets[i++] as number; - const privateName = targets[i++] as string; - ngDevMode && assertIndexInRange(lView, index); - const instance = lView[index]; - const def = tView.data[index] as DirectiveDef; - - writeToDirectiveInput(def, instance, propName, privateName, value); - } - - const element = getNativeByTNode(tNode, lView) as RElement | RComment; - if (isComponentHost(tNode)) { - markDirtyIfOnPush(lView, tNode.index); - } - if (ngDevMode) { - setNgReflectProperties(lView, element, tNode.type, targets, value); - } -} diff --git a/packages/core/src/render3/instructions/property_iterpolation_create.ts b/packages/core/src/render3/instructions/property_iterpolation_create.ts deleted file mode 100644 index cc6140f84c4..00000000000 --- a/packages/core/src/render3/instructions/property_iterpolation_create.ts +++ /dev/null @@ -1,131 +0,0 @@ -/** - * @license - * Copyright Google LLC All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ - -import {computed} from '../../render3/reactivity/computed'; -import {assertDefined, assertIndexInRange} from '../../util/assert'; -import {EMPTY_ARRAY} from '../../util/empty'; -import {DirectiveDef} from '../interfaces/definition'; -import {PropertyAliasValue, TNode} from '../interfaces/node'; -import {SanitizerFn} from '../interfaces/sanitization'; -import {isComponentHost} from '../interfaces/type_checks'; -import {HEADER_OFFSET, RENDERER, TVIEW} from '../interfaces/view'; -import {getCurrentTNode, getLView, getSelectedTNode, getTView} from '../state'; -import {NO_CHANGE} from '../tokens'; -import {renderStringify} from '../util/stringify_utils'; - -import {interpolationV} from './interpolation'; -import {determineInputTargets} from './property_create_shared'; -import {elementPropertyInternal, markDirtyIfOnPush, writeToDirectiveInput} from './shared'; - - -function concatenateInterpolatedValue(values: any[]): string { - // Build the updated content - let content = values[0]; - for (let i = 1; i < values.length; i += 2) { - content += renderStringify(values[i]) + values[i + 1]; - } - - return content; -} - -/** - * TODO - * - * @codeGenApi - */ -export function ɵɵpropertyInterpolateVCreate( - slot: number, propName: string, staticParts: string[], values: any[], - sanitizer?: SanitizerFn|null): typeof ɵɵpropertyInterpolateVCreate { - const lView = getLView(); - const expressionsSlot = HEADER_OFFSET + slot; - - const tView = getTView(); - const tNode = getCurrentTNode(); - assertDefined(tNode, `propertyCreate() must follow an actual element`); - - const inputData = tNode.inputs?.[propName] ?? EMPTY_ARRAY; - const {zoneTargets, signalInputs} = determineInputTargets(inputData, tView, lView); - - // If there are multiple signal targets, or any zone targets, then wrap `expr` in a computed. This - // ensures that the interpolated string is concatenated only once. - let exprValueComputed: (() => string)|null = null; - if (zoneTargets.length > 0 || signalInputs.length > 1) { - exprValueComputed = computed(() => concatenateInterpolatedValue(values)); - } - - lView[expressionsSlot] = values; - for (const inputSignal of signalInputs) { - inputSignal.bind(inputSignal, {computation: exprValueComputed!}); - } - - if (tView.firstCreatePass) { - if (inputData.length === 0) { - // Untargeted input -> DOM binding. - (tView.virtualUpdate ??= []).push({ - slot: expressionsSlot, - instruction: () => propertyInterpolationUpdateDom( - tNode.index, propName, expressionsSlot, sanitizer ?? null), - }); - } else if (zoneTargets?.length ?? 0 > 0) { - // Some binding targets were zone-based, so we need an update instruction to process them. - (tView.virtualUpdate ??= []).push({ - slot: expressionsSlot, - instruction: () => - propertyInterpolationUpdateInput(propName, expressionsSlot, zoneTargets!), - }); - } else { - // The only target(s) were signal-based, so no update path is needed. - } - } - - return ɵɵpropertyInterpolateVCreate; -} -function propertyInterpolationUpdateDom( - nodeSlot: number, propName: string, expressionSlot: number, sanitizer: SanitizerFn|null): void { - const lView = getLView(); - const values: any[] = lView[expressionSlot]; - const interpolatedValue = interpolationV(lView, values); - if (interpolatedValue !== NO_CHANGE) { - const tView = getTView(); - const tNode = tView.data[nodeSlot] as TNode; - elementPropertyInternal( - tView, tNode, lView, propName, interpolatedValue, lView[RENDERER], sanitizer, false, - /* TODO(signals) */ false); - - // TODO(pk): ngDevMode part of this instruction - } -} - -export function propertyInterpolationUpdateInput( - propName: string, expressionSlot: number, targets: PropertyAliasValue): void { - const lView = getLView(); - const values: any[] = lView[expressionSlot]; - const interpolatedValue = interpolationV(lView, values); - if (interpolatedValue !== NO_CHANGE) { - // TODO(pk): inconsistent signature of update instructions - const tNode = getSelectedTNode(); - const tView = lView[TVIEW]; - ngDevMode && assertDefined(tNode.inputs, `Expected tNode to have inputs`); - - for (let i = 0; i < targets.length;) { - const index = targets[i++] as number; - const privateName = targets[i++] as string; - ngDevMode && assertIndexInRange(lView, index); - const instance = lView[index]; - const def = tView.data[index] as DirectiveDef; - - writeToDirectiveInput(def, instance, propName, privateName, interpolatedValue as string); - - if (isComponentHost(tNode)) { - markDirtyIfOnPush(lView, tNode.index); - } - - // TODO(pk): ngDevMode part of the inputs update - } - } -} diff --git a/packages/core/test/acceptance/signal-components/input_interpolation.spec.ts b/packages/core/test/acceptance/signal-components/input_interpolation.spec.ts index 3f303a6f0c7..693fb4b6b57 100644 --- a/packages/core/test/acceptance/signal-components/input_interpolation.spec.ts +++ b/packages/core/test/acceptance/signal-components/input_interpolation.spec.ts @@ -34,7 +34,7 @@ describe('Signal component input interpolations', () => { xit('should bind interpolated values', () => { @Component({ signals: true, - template: ``, + template: ``, imports: [Print], standalone: true, })