Skip to content

Commit

Permalink
refactor(ivy): remove superfluous bind() function (angular#32489)
Browse files Browse the repository at this point in the history
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 angular#32489
  • Loading branch information
pkozlowski-opensource authored and matsko committed Sep 5, 2019
1 parent fed6b25 commit 5ab7cb4
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 34 deletions.
14 changes: 7 additions & 7 deletions packages/core/src/render3/i18n.md
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand All @@ -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 `<div i18n>...</div>`
}
}
Expand Down Expand Up @@ -601,7 +601,7 @@ template: function(rf: RenderFlags, ctx: MyComponent) {
elementEnd();
}
if (rf & RenderFlags.Update) {
i18nExp(bind(ctx.count));
i18nExp(ctx.count);
i18nApply(1);
}
}
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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);
}
}
Expand Down
9 changes: 5 additions & 4 deletions packages/core/src/render3/i18n.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';

Expand Down Expand Up @@ -1031,8 +1033,7 @@ let shiftsCounter = 0;
*/
export function ɵɵi18nExp<T>(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++;
Expand Down
10 changes: 4 additions & 6 deletions packages/core/src/render3/instructions/attribute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';


Expand All @@ -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;
}
15 changes: 2 additions & 13 deletions packages/core/src/render3/instructions/property.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -42,14 +41,4 @@ export function ɵɵproperty<T>(
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<T>(lView: LView, value: T): T|NO_CHANGE {
return bindingUpdated(lView, lView[BINDING_INDEX]++, value) ? value : NO_CHANGE;
}
}
8 changes: 4 additions & 4 deletions packages/core/src/render3/interfaces/i18n.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
* }
* ```
Expand Down

0 comments on commit 5ab7cb4

Please sign in to comment.