Skip to content

Commit

Permalink
feat(ivy): add property instruction (#29513)
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
benlesh authored and mhevery committed Mar 27, 2019
1 parent 1e5a818 commit e4c1c88
Show file tree
Hide file tree
Showing 3 changed files with 185 additions and 7 deletions.
70 changes: 65 additions & 5 deletions packages/core/src/render3/instructions/instructions.ts
Expand Up @@ -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';
Expand All @@ -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';
Expand Down Expand Up @@ -408,6 +408,10 @@ export function renderEmbeddedTemplate<T>(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
Expand Down Expand Up @@ -453,6 +457,10 @@ function renderComponentOrTemplate<T>(
// 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);
}

Expand Down Expand Up @@ -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 <div/> 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);
}
Expand Down Expand Up @@ -1142,6 +1169,34 @@ export function elementAttribute(
}

/**
* 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<T>(
propName: string, value: T, sanitizer?: SanitizerFn | null,
nativeOnly?: boolean): typeof property {
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,
Expand Down Expand Up @@ -2648,7 +2703,12 @@ export function checkView<T>(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) {
Expand Down
27 changes: 26 additions & 1 deletion packages/core/src/render3/state.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}
95 changes: 94 additions & 1 deletion packages/core/test/render3/instructions_spec.ts
Expand Up @@ -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';
Expand All @@ -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() {
Expand Down Expand Up @@ -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', () => {
// <div [title]="title"></div>
const t = new TemplateFixture(createDiv, () => {}, 1, 1);
t.update(() => {
select(0);
property('title', 'one');
});
expect(t.html).toEqual('<div title="one"></div>');
t.update(() => {
select(0);
property('title', 'two');
});
expect(t.html).toEqual('<div title="two"></div>');
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', () => {
// <div [title]="title" [accesskey]="key"></div>
const t = new TemplateFixture(createDiv, () => {}, 1, 2);
t.update(() => {
select(0);
property('title', 'one')('accessKey', 'A');
});
expect(t.html).toEqual('<div accesskey="A" title="one"></div>');
t.update(() => {
select(0);
property('title', 'two')('accessKey', 'B');
});
expect(t.html).toEqual('<div accesskey="B" title="two"></div>');
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', () => {
// <div [title]="title" [accesskey]="key"></div>
const t = new TemplateFixture(createDiv, () => {}, 1, 2);
t.update(() => {
select(0);
property('title', 'one')('accessKey', 'A');
});
expect(t.html).toEqual('<div accesskey="A" title="one"></div>');
t.update(() => {
select(0);
property('title', 'two')('accessKey', 'A'); // Notice: only changing the title.
});
expect(t.html).toEqual('<div accesskey="A" title="two"></div>');
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);
Expand Down

0 comments on commit e4c1c88

Please sign in to comment.