From 5ab7cb418874c99833061ac20797a8d7ec57954a Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Thu, 5 Sep 2019 11:16:56 +0200 Subject: [PATCH] refactor(ivy): remove superfluous bind() function (#32489) Historically bind() used to be a separate instruction. With a series of refactoring it became a utility function but after recent code changes it does not provide any valuable abstraction / help. On the contrary - it can be seen as a performance problem (forces unnecessary comparison to `NO_CHANGE` during change detection). PR Close #32489 --- packages/core/src/render3/i18n.md | 14 +++++++------- packages/core/src/render3/i18n.ts | 9 +++++---- .../core/src/render3/instructions/attribute.ts | 10 ++++------ .../core/src/render3/instructions/property.ts | 15 ++------------- packages/core/src/render3/interfaces/i18n.ts | 8 ++++---- 5 files changed, 22 insertions(+), 34 deletions(-) diff --git a/packages/core/src/render3/i18n.md b/packages/core/src/render3/i18n.md index 902790c8c2c5f..f079fe2a22d2a 100644 --- a/packages/core/src/render3/i18n.md +++ b/packages/core/src/render3/i18n.md @@ -67,7 +67,7 @@ function MyComponent_NgIf_Template_0(rf: RenderFlags, ctx: any) { i18nEnd(); } if (rf & RenderFlags.Update) { - i18nExp(bind(ctx.count)); // referenced by `�0:1�` + i18nExp(ctx.count); // referenced by `�0:1�` i18nApply(0); } } @@ -85,9 +85,9 @@ class MyComponent { elementEnd(); } if (rf & RenderFlags.Update) { - i18nExp(bind(ctx.name)); // referenced by `�0�` in `MSG_title` + i18nExp(ctx.name); // referenced by `�0�` in `MSG_title` i18nApply(1); // Updates the `i18n-title` binding - i18nExp(bind(ctx.count)); // referenced by `�0�` in `MSG_div` + i18nExp(ctx.count); // referenced by `�0�` in `MSG_div` i18nApply(2); // Updates the `
...
` } } @@ -601,7 +601,7 @@ template: function(rf: RenderFlags, ctx: MyComponent) { elementEnd(); } if (rf & RenderFlags.Update) { - i18nExp(bind(ctx.count)); + i18nExp(ctx.count); i18nApply(1); } } @@ -679,7 +679,7 @@ class MyComponent { elementEnd(); } if (rf & RenderFlags.Update) { - i18nExp(bind(ctx.count)); // referenced by `�0�` + i18nExp(ctx.count); // referenced by `�0�` i18nApply(1); // Updates the `i18n-title` binding } } @@ -919,8 +919,8 @@ class MyComponent { i18nEnd(); } if (rf & RenderFlags.Update) { - i18nExp(bind(ctx.count)); // referenced by `�0�` - i18nExp(bind(ctx.animal)); // referenced by `�1�` + i18nExp(ctx.count); // referenced by `�0�` + i18nExp(ctx.animal); // referenced by `�1�` i18nApply(0); } } diff --git a/packages/core/src/render3/i18n.ts b/packages/core/src/render3/i18n.ts index 34cc2757b29ec..3a5592c5c77ca 100644 --- a/packages/core/src/render3/i18n.ts +++ b/packages/core/src/render3/i18n.ts @@ -6,14 +6,17 @@ * found in the LICENSE file at https://angular.io/license */ import '../util/ng_i18n_closure_mode'; + import {DEFAULT_LOCALE_ID, getPluralCase} from '../i18n/localization'; import {SRCSET_ATTRS, URI_ATTRS, VALID_ATTRS, VALID_ELEMENTS, getTemplateContent} from '../sanitization/html_sanitizer'; import {InertBodyHelper} from '../sanitization/inert_body'; import {_sanitizeUrl, sanitizeSrcset} from '../sanitization/url_sanitizer'; import {addAllToArray} from '../util/array_utils'; import {assertDataInRange, assertDefined, assertEqual, assertGreaterThan} from '../util/assert'; + +import {bindingUpdated} from './bindings'; import {attachPatchData} from './context_discovery'; -import {bind, setDelayProjection} from './instructions/all'; +import {setDelayProjection} from './instructions/all'; import {attachI18nOpCodesDebug} from './instructions/lview_debug'; import {TsickleIssue1009, allocExpando, elementAttributeInternal, elementPropertyInternal, getOrCreateTNode, setInputsForProperty, textBindingInternal} from './instructions/shared'; import {LContainer, NATIVE} from './interfaces/container'; @@ -25,7 +28,6 @@ import {isLContainer} from './interfaces/type_checks'; import {BINDING_INDEX, HEADER_OFFSET, LView, RENDERER, TVIEW, TView, T_HOST} from './interfaces/view'; import {appendChild, applyProjection, createTextNode, nativeRemoveNode} from './node_manipulation'; import {getIsParent, getLView, getPreviousOrParentTNode, setIsNotParent, setPreviousOrParentTNode} from './state'; -import {NO_CHANGE} from './tokens'; import {renderStringify} from './util/misc_utils'; import {getNativeByIndex, getNativeByTNode, getTNode, load} from './util/view_utils'; @@ -1031,8 +1033,7 @@ let shiftsCounter = 0; */ export function ɵɵi18nExp(value: T): TsickleIssue1009 { const lView = getLView(); - const expression = bind(lView, value); - if (expression !== NO_CHANGE) { + if (bindingUpdated(lView, lView[BINDING_INDEX]++, value)) { changeMask = changeMask | (1 << shiftsCounter); } shiftsCounter++; diff --git a/packages/core/src/render3/instructions/attribute.ts b/packages/core/src/render3/instructions/attribute.ts index 3589ba27ada01..3cb12ffd73169 100644 --- a/packages/core/src/render3/instructions/attribute.ts +++ b/packages/core/src/render3/instructions/attribute.ts @@ -5,11 +5,11 @@ * 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 {bindingUpdated} from '../bindings'; import {SanitizerFn} from '../interfaces/sanitization'; +import {BINDING_INDEX} from '../interfaces/view'; import {getLView, getSelectedIndex} from '../state'; -import {NO_CHANGE} from '../tokens'; -import {bind} from './property'; import {TsickleIssue1009, elementAttributeInternal} from './shared'; @@ -30,11 +30,9 @@ import {TsickleIssue1009, elementAttributeInternal} from './shared'; export function ɵɵattribute( name: string, value: any, sanitizer?: SanitizerFn | null, namespace?: string): TsickleIssue1009 { - const index = getSelectedIndex(); const lView = getLView(); - const bound = bind(lView, value); - if (bound !== NO_CHANGE) { - elementAttributeInternal(index, name, bound, lView, sanitizer, namespace); + if (bindingUpdated(lView, lView[BINDING_INDEX]++, value)) { + elementAttributeInternal(getSelectedIndex(), name, value, lView, sanitizer, namespace); } return ɵɵattribute; } diff --git a/packages/core/src/render3/instructions/property.ts b/packages/core/src/render3/instructions/property.ts index 0ae3e0537e837..9cf7bba9f4f04 100644 --- a/packages/core/src/render3/instructions/property.ts +++ b/packages/core/src/render3/instructions/property.ts @@ -7,9 +7,8 @@ */ import {bindingUpdated} from '../bindings'; import {SanitizerFn} from '../interfaces/sanitization'; -import {BINDING_INDEX, LView, TVIEW} from '../interfaces/view'; +import {BINDING_INDEX, TVIEW} from '../interfaces/view'; import {getLView, getSelectedIndex} from '../state'; -import {NO_CHANGE} from '../tokens'; import {TsickleIssue1009, elementPropertyInternal, storePropertyBindingMetadata} from './shared'; @@ -42,14 +41,4 @@ export function ɵɵproperty( ngDevMode && storePropertyBindingMetadata(lView[TVIEW].data, nodeIndex, propName, bindingIndex); } return ɵɵproperty; -} - -/** - * Creates a single value binding. - * - * @param lView Current view - * @param value Value to diff - */ -export function bind(lView: LView, value: T): T|NO_CHANGE { - return bindingUpdated(lView, lView[BINDING_INDEX]++, value) ? value : NO_CHANGE; -} +} \ No newline at end of file diff --git a/packages/core/src/render3/interfaces/i18n.ts b/packages/core/src/render3/interfaces/i18n.ts index c97e8d8211cd5..98da1ca9c4298 100644 --- a/packages/core/src/render3/interfaces/i18n.ts +++ b/packages/core/src/render3/interfaces/i18n.ts @@ -200,10 +200,10 @@ export const enum I18nUpdateOpCode { * Assume * ```ts * if (rf & RenderFlags.Update) { - * i18nExp(bind(ctx.exp1)); // If changed set mask bit 1 - * i18nExp(bind(ctx.exp2)); // If changed set mask bit 2 - * i18nExp(bind(ctx.exp3)); // If changed set mask bit 3 - * i18nExp(bind(ctx.exp4)); // If changed set mask bit 4 + * i18nExp(ctx.exp1); // If changed set mask bit 1 + * i18nExp(ctx.exp2); // If changed set mask bit 2 + * i18nExp(ctx.exp3); // If changed set mask bit 3 + * i18nExp(ctx.exp4); // If changed set mask bit 4 * i18nApply(0); // Apply all changes by executing the OpCodes. * } * ```