From 33963ca0d3ebe93c07dced70d63d932769ca4077 Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Tue, 26 Mar 2019 14:57:36 -0700 Subject: [PATCH] feat(ivy): add property instruction (#29513) - Adds `property` instruction - Does _NOT_ add compiler changes to accommodate `property` instruction, that will be a follow up PR. - Updates `select` instruction to set the selected index in state. - Adds dev mode assertions around the selected index state. Related #29527 PR Close #29513 --- .../src/render3/instructions/instructions.ts | 77 ++++++++++++++- packages/core/src/render3/state.ts | 27 +++++- .../core/test/render3/instructions_spec.ts | 95 ++++++++++++++++++- 3 files changed, 192 insertions(+), 7 deletions(-) diff --git a/packages/core/src/render3/instructions/instructions.ts b/packages/core/src/render3/instructions/instructions.ts index 99b62a224f..bd9cb4245f 100644 --- a/packages/core/src/render3/instructions/instructions.ts +++ b/packages/core/src/render3/instructions/instructions.ts @@ -13,7 +13,7 @@ import {Type} from '../../interface/type'; import {CUSTOM_ELEMENTS_SCHEMA, NO_ERRORS_SCHEMA, SchemaMetadata} from '../../metadata/schema'; import {validateAgainstEventAttributes, validateAgainstEventProperties} from '../../sanitization/sanitization'; import {Sanitizer} from '../../sanitization/security'; -import {assertDataInRange, assertDefined, assertDomNode, assertEqual, assertLessThan, assertNotEqual} from '../../util/assert'; +import {assertDataInRange, assertDefined, assertDomNode, assertEqual, assertGreaterThan, assertLessThan, assertNotEqual} from '../../util/assert'; import {isObservable} from '../../util/lang'; import {normalizeDebugBindingName, normalizeDebugBindingValue} from '../../util/ng_reflect'; import {assertHasParent, assertLContainerOrUndefined, assertLView, assertPreviousIsParent} from '../assert'; @@ -37,7 +37,7 @@ import {assertNodeOfPossibleTypes, assertNodeType} from '../node_assert'; import {appendChild, appendProjectedNodes, createTextNode, insertView, removeView} from '../node_manipulation'; import {isNodeMatchingSelectorList, matchingProjectionSelectorIndex} from '../node_selector_matcher'; import {applyOnCreateInstructions} from '../node_util'; -import {decreaseElementDepthCount, enterView, getActiveHostContext, getBindingsEnabled, getCheckNoChangesMode, getContextLView, getCurrentDirectiveDef, getElementDepthCount, getIsParent, getLView, getPreviousOrParentTNode, increaseElementDepthCount, isCreationMode, leaveView, nextContextImpl, resetComponentState, setActiveHost, setBindingRoot, setCheckNoChangesMode, setCurrentDirectiveDef, setCurrentQueryIndex, setIsParent, setPreviousOrParentTNode} from '../state'; +import {decreaseElementDepthCount, enterView, getActiveHostContext, getBindingsEnabled, getCheckNoChangesMode, getContextLView, getCurrentDirectiveDef, getElementDepthCount, getIsParent, getLView, getPreviousOrParentTNode, getSelectedIndex, increaseElementDepthCount, isCreationMode, leaveView, nextContextImpl, resetComponentState, setActiveHost, setBindingRoot, setCheckNoChangesMode, setCurrentDirectiveDef, setCurrentQueryIndex, setIsParent, setPreviousOrParentTNode, setSelectedIndex} from '../state'; import {getInitialClassNameValue, getInitialStyleStringValue, initializeStaticContext as initializeStaticStylingContext, patchContextWithStaticAttrs, renderInitialClasses, renderInitialStyles} from '../styling/class_and_style_bindings'; import {ANIMATION_PROP_PREFIX, getStylingContext, hasClassInput, hasStyleInput, isAnimationProp} from '../styling/util'; import {NO_CHANGE} from '../tokens'; @@ -408,6 +408,10 @@ export function renderEmbeddedTemplate(viewToRender: LView, tView: TView, con oldView = enterView(viewToRender, viewToRender[T_HOST]); resetPreOrderHookFlags(viewToRender); namespaceHTML(); + + // Reset the selected index so we can assert that `select` was called later + ngDevMode && setSelectedIndex(-1); + tView.template !(getRenderFlags(viewToRender), context); // This must be set to false immediately after the first creation run because in an // ngFor loop, all the views will be created together before update mode runs and turns @@ -453,6 +457,10 @@ function renderComponentOrTemplate( // creation mode pass if (templateFn) { namespaceHTML(); + + // Reset the selected index so we can assert that `select` was called later + ngDevMode && setSelectedIndex(-1); + templateFn(RenderFlags.Create, context); } @@ -1093,11 +1101,30 @@ export function elementEnd(): void { /** - * Flushes all the lifecycle hooks for directives up until (and excluding) that node index + * Selects an index of an item to act on and flushes lifecycle hooks up to this point * - * @param index The index of the element in the `LView` - */ + * Used in conjunction with instructions like {@link property} to act on elements with specified + * indices, for example those created with {@link element} or {@link elementStart}. + * + * ```ts + * (rf: RenderFlags, ctx: any) => { + * if (rf & 1) { + * element(0, 'div'); + * } + * if (rf & 2) { + * select(0); // Select the
created above. + * property('title', 'test'); + * } + * } + * ``` + * @param index the index of the item to act on with the following instructions + */ export function select(index: number): void { + ngDevMode && assertGreaterThan(index, -1, 'Invalid index'); + ngDevMode && + assertLessThan( + index, getLView().length - HEADER_OFFSET, 'Should be within range for the view data'); + setSelectedIndex(index); const lView = getLView(); executePreOrderHooks(lView, lView[TVIEW], getCheckNoChangesMode(), index); } @@ -1141,7 +1168,42 @@ export function elementAttribute( } } +// TODO: Remove this when the issue is resolved. /** + * Tsickle freaked out over a function returning itself + * https://github.com/angular/tsickle/issues/1009) + */ +export type TsickleIssue1009 = any; + +/** + * Update a property on a selected element. + * + * Operates on the element selected by index via the {@link select} instruction. + * + * If the property name also exists as an input property on one of the element's directives, + * the component property will be set instead of the element property. This check must + * be conducted at runtime so child components that add new `@Inputs` don't have to be re-compiled + * + * @param propName Name of property. Because it is going to DOM, this is not subject to + * renaming as part of minification. + * @param value New value to write. + * @param sanitizer An optional function used to sanitize the value. + * @param nativeOnly Whether or not we should only set native properties and skip input check + * (this is necessary for host property bindings) + * @returns This function returns itself so that it may be chained + * (e.g. `property('name', ctx.name)('title', ctx.title)`) + */ +export function property( + propName: string, value: T, sanitizer?: SanitizerFn | null, + nativeOnly?: boolean): TsickleIssue1009 { + const index = getSelectedIndex(); + const bindReconciledValue = bind(value); + elementPropertyInternal(index, propName, bindReconciledValue, sanitizer, nativeOnly); + return property; +} + +/** + * **TODO: Remove this function after `property` is in use** * Update a property on an element. * * If the property name also exists as an input property on one of the element's directives, @@ -2648,7 +2710,12 @@ export function checkView(hostView: LView, component: T) { resetPreOrderHookFlags(hostView); namespaceHTML(); creationMode && executeViewQueryFn(RenderFlags.Create, hostTView, component); + + // Reset the selected index so we can assert that `select` was called later + ngDevMode && setSelectedIndex(-1); + templateFn(getRenderFlags(hostView), component); + refreshDescendantViews(hostView); // Only check view queries again in creation mode if there are static view queries if (!creationMode || hostTView.staticViewQueries) { diff --git a/packages/core/src/render3/state.ts b/packages/core/src/render3/state.ts index 62095887c1..41bda69c45 100644 --- a/packages/core/src/render3/state.ts +++ b/packages/core/src/render3/state.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {assertDefined} from '../util/assert'; +import {assertDefined, assertGreaterThan} from '../util/assert'; import {assertLViewOrUndefined} from './assert'; import {executeHooks} from './hooks'; @@ -340,3 +340,28 @@ export function leaveView(newView: LView): void { } enterView(newView, null); } + +let _selectedIndex = -1; + +/** + * Gets the most recent index passed to {@link select} + * + * Used with {@link property} instruction (and more in the future) to identify the index in the + * current `LView` to act on. + */ +export function getSelectedIndex() { + ngDevMode && + assertGreaterThan( + _selectedIndex, -1, 'select() should be called prior to retrieving the selected index'); + return _selectedIndex; +} + +/** + * Sets the most recent index passed to {@link select} + * + * Used with {@link property} instruction (and more in the future) to identify the index in the + * current `LView` to act on. + */ +export function setSelectedIndex(index: number) { + _selectedIndex = index; +} diff --git a/packages/core/test/render3/instructions_spec.ts b/packages/core/test/render3/instructions_spec.ts index 49b8a7c030..51926fb1cb 100644 --- a/packages/core/test/render3/instructions_spec.ts +++ b/packages/core/test/render3/instructions_spec.ts @@ -10,7 +10,7 @@ import {NgForOfContext} from '@angular/common'; import {RenderFlags} from '../../src/render3'; import {defineComponent} from '../../src/render3/definition'; -import {bind, element, elementAttribute, elementEnd, elementProperty, elementStart, elementStyleProp, elementStyling, elementStylingApply, elementStylingMap, interpolation1, renderTemplate, template, text, textBinding} from '../../src/render3/instructions/all'; +import {bind, element, elementAttribute, elementEnd, elementProperty, elementStart, elementStyleProp, elementStyling, elementStylingApply, elementStylingMap, interpolation1, renderTemplate, template, text, textBinding, select, property} from '../../src/render3/instructions/all'; import {AttributeMarker} from '../../src/render3/interfaces/node'; import {bypassSanitizationTrustHtml, bypassSanitizationTrustResourceUrl, bypassSanitizationTrustScript, bypassSanitizationTrustStyle, bypassSanitizationTrustUrl} from '../../src/sanitization/bypass'; import {defaultStyleSanitizer, sanitizeHtml, sanitizeResourceUrl, sanitizeScript, sanitizeStyle, sanitizeUrl} from '../../src/sanitization/sanitization'; @@ -19,6 +19,7 @@ import {StyleSanitizeFn} from '../../src/sanitization/style_sanitizer'; import {NgForOf} from './common_with_def'; import {ComponentFixture, TemplateFixture} from './render_util'; +import {setSelectedIndex, getSelectedIndex} from '@angular/core/src/render3/state'; describe('instructions', () => { function createAnchor() { @@ -162,6 +163,98 @@ describe('instructions', () => { }); }); + describe('select', () => { + it('should error in DevMode if index is out of range', () => { + // Only one constant added, meaning only index `0` is valid. + const t = new TemplateFixture(createDiv, () => {}, 1, 0); + expect(() => { t.update(() => { select(-1); }); }).toThrow(); + expect(() => { t.update(() => { select(1); }); }).toThrow(); + expect(() => { t.update(() => { select(0); }); }).not.toThrow(); + }); + }); + + describe('property', () => { + // TODO(benlesh): Replace with TestBed tests once the instruction is being generated. + it('should set properties of the selected element', () => { + //
+ const t = new TemplateFixture(createDiv, () => {}, 1, 1); + t.update(() => { + select(0); + property('title', 'one'); + }); + expect(t.html).toEqual('
'); + t.update(() => { + select(0); + property('title', 'two'); + }); + expect(t.html).toEqual('
'); + expect(ngDevMode).toHaveProperties({ + firstTemplatePass: 1, + tNode: 2, // 1 for div, 1 for host element + tView: 2, // 1 for rootView + 1 for the template view + rendererCreateElement: 1, + rendererSetProperty: 2, + }); + }); + + // TODO(benlesh): Replace with TestBed tests once the instruction is being generated. + it('should chain', () => { + //
+ const t = new TemplateFixture(createDiv, () => {}, 1, 2); + t.update(() => { + select(0); + property('title', 'one')('accessKey', 'A'); + }); + expect(t.html).toEqual('
'); + t.update(() => { + select(0); + property('title', 'two')('accessKey', 'B'); + }); + expect(t.html).toEqual('
'); + expect(ngDevMode).toHaveProperties({ + firstTemplatePass: 1, + tNode: 2, // 1 for div, 1 for host element + tView: 2, // 1 for rootView + 1 for the template view + rendererCreateElement: 1, + rendererSetProperty: 4, + }); + }); + + // TODO(benlesh): Replace with TestBed tests once the instruction is being generated. + it('should diff value changes', () => { + //
+ const t = new TemplateFixture(createDiv, () => {}, 1, 2); + t.update(() => { + select(0); + property('title', 'one')('accessKey', 'A'); + }); + expect(t.html).toEqual('
'); + t.update(() => { + select(0); + property('title', 'two')('accessKey', 'A'); // Notice: only changing the title. + }); + expect(t.html).toEqual('
'); + expect(ngDevMode).toHaveProperties({ + firstTemplatePass: 1, + tNode: 2, // 1 for div, 1 for host element + tView: 2, // 1 for rootView + 1 for the template view + rendererCreateElement: 1, + rendererSetProperty: 3, + }); + }); + + it('should error in dev mode if select was not called prior', () => { + const t = new TemplateFixture(createDiv, () => {}, 1, 1); + expect(() => { t.update(() => { property('title', 'test'); }); }).toThrow(); + expect(() => { + t.update(() => { + select(0); + property('title', 'test'); + }); + }).not.toThrow(); + }); + }); + describe('elementProperty', () => { it('should use sanitizer function when available', () => { const t = new TemplateFixture(createDiv, () => {}, 1);