Skip to content

Commit 33963ca

Browse files
benleshjasonaden
authored andcommitted
feat(ivy): add property instruction (angular#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 angular#29527 PR Close angular#29513
1 parent 7b27009 commit 33963ca

File tree

3 files changed

+192
-7
lines changed

3 files changed

+192
-7
lines changed

packages/core/src/render3/instructions/instructions.ts

Lines changed: 72 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {Type} from '../../interface/type';
1313
import {CUSTOM_ELEMENTS_SCHEMA, NO_ERRORS_SCHEMA, SchemaMetadata} from '../../metadata/schema';
1414
import {validateAgainstEventAttributes, validateAgainstEventProperties} from '../../sanitization/sanitization';
1515
import {Sanitizer} from '../../sanitization/security';
16-
import {assertDataInRange, assertDefined, assertDomNode, assertEqual, assertLessThan, assertNotEqual} from '../../util/assert';
16+
import {assertDataInRange, assertDefined, assertDomNode, assertEqual, assertGreaterThan, assertLessThan, assertNotEqual} from '../../util/assert';
1717
import {isObservable} from '../../util/lang';
1818
import {normalizeDebugBindingName, normalizeDebugBindingValue} from '../../util/ng_reflect';
1919
import {assertHasParent, assertLContainerOrUndefined, assertLView, assertPreviousIsParent} from '../assert';
@@ -37,7 +37,7 @@ import {assertNodeOfPossibleTypes, assertNodeType} from '../node_assert';
3737
import {appendChild, appendProjectedNodes, createTextNode, insertView, removeView} from '../node_manipulation';
3838
import {isNodeMatchingSelectorList, matchingProjectionSelectorIndex} from '../node_selector_matcher';
3939
import {applyOnCreateInstructions} from '../node_util';
40-
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';
40+
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';
4141
import {getInitialClassNameValue, getInitialStyleStringValue, initializeStaticContext as initializeStaticStylingContext, patchContextWithStaticAttrs, renderInitialClasses, renderInitialStyles} from '../styling/class_and_style_bindings';
4242
import {ANIMATION_PROP_PREFIX, getStylingContext, hasClassInput, hasStyleInput, isAnimationProp} from '../styling/util';
4343
import {NO_CHANGE} from '../tokens';
@@ -408,6 +408,10 @@ export function renderEmbeddedTemplate<T>(viewToRender: LView, tView: TView, con
408408
oldView = enterView(viewToRender, viewToRender[T_HOST]);
409409
resetPreOrderHookFlags(viewToRender);
410410
namespaceHTML();
411+
412+
// Reset the selected index so we can assert that `select` was called later
413+
ngDevMode && setSelectedIndex(-1);
414+
411415
tView.template !(getRenderFlags(viewToRender), context);
412416
// This must be set to false immediately after the first creation run because in an
413417
// ngFor loop, all the views will be created together before update mode runs and turns
@@ -453,6 +457,10 @@ function renderComponentOrTemplate<T>(
453457
// creation mode pass
454458
if (templateFn) {
455459
namespaceHTML();
460+
461+
// Reset the selected index so we can assert that `select` was called later
462+
ngDevMode && setSelectedIndex(-1);
463+
456464
templateFn(RenderFlags.Create, context);
457465
}
458466

@@ -1093,11 +1101,30 @@ export function elementEnd(): void {
10931101

10941102

10951103
/**
1096-
* Flushes all the lifecycle hooks for directives up until (and excluding) that node index
1104+
* Selects an index of an item to act on and flushes lifecycle hooks up to this point
10971105
*
1098-
* @param index The index of the element in the `LView`
1099-
*/
1106+
* Used in conjunction with instructions like {@link property} to act on elements with specified
1107+
* indices, for example those created with {@link element} or {@link elementStart}.
1108+
*
1109+
* ```ts
1110+
* (rf: RenderFlags, ctx: any) => {
1111+
* if (rf & 1) {
1112+
* element(0, 'div');
1113+
* }
1114+
* if (rf & 2) {
1115+
* select(0); // Select the <div/> created above.
1116+
* property('title', 'test');
1117+
* }
1118+
* }
1119+
* ```
1120+
* @param index the index of the item to act on with the following instructions
1121+
*/
11001122
export function select(index: number): void {
1123+
ngDevMode && assertGreaterThan(index, -1, 'Invalid index');
1124+
ngDevMode &&
1125+
assertLessThan(
1126+
index, getLView().length - HEADER_OFFSET, 'Should be within range for the view data');
1127+
setSelectedIndex(index);
11011128
const lView = getLView();
11021129
executePreOrderHooks(lView, lView[TVIEW], getCheckNoChangesMode(), index);
11031130
}
@@ -1141,7 +1168,42 @@ export function elementAttribute(
11411168
}
11421169
}
11431170

1171+
// TODO: Remove this when the issue is resolved.
1172+
/**
1173+
* Tsickle freaked out over a function returning itself
1174+
* https://github.com/angular/tsickle/issues/1009)
1175+
*/
1176+
export type TsickleIssue1009 = any;
1177+
1178+
/**
1179+
* Update a property on a selected element.
1180+
*
1181+
* Operates on the element selected by index via the {@link select} instruction.
1182+
*
1183+
* If the property name also exists as an input property on one of the element's directives,
1184+
* the component property will be set instead of the element property. This check must
1185+
* be conducted at runtime so child components that add new `@Inputs` don't have to be re-compiled
1186+
*
1187+
* @param propName Name of property. Because it is going to DOM, this is not subject to
1188+
* renaming as part of minification.
1189+
* @param value New value to write.
1190+
* @param sanitizer An optional function used to sanitize the value.
1191+
* @param nativeOnly Whether or not we should only set native properties and skip input check
1192+
* (this is necessary for host property bindings)
1193+
* @returns This function returns itself so that it may be chained
1194+
* (e.g. `property('name', ctx.name)('title', ctx.title)`)
1195+
*/
1196+
export function property<T>(
1197+
propName: string, value: T, sanitizer?: SanitizerFn | null,
1198+
nativeOnly?: boolean): TsickleIssue1009 {
1199+
const index = getSelectedIndex();
1200+
const bindReconciledValue = bind(value);
1201+
elementPropertyInternal(index, propName, bindReconciledValue, sanitizer, nativeOnly);
1202+
return property;
1203+
}
1204+
11441205
/**
1206+
* **TODO: Remove this function after `property` is in use**
11451207
* Update a property on an element.
11461208
*
11471209
* If the property name also exists as an input property on one of the element's directives,
@@ -2648,7 +2710,12 @@ export function checkView<T>(hostView: LView, component: T) {
26482710
resetPreOrderHookFlags(hostView);
26492711
namespaceHTML();
26502712
creationMode && executeViewQueryFn(RenderFlags.Create, hostTView, component);
2713+
2714+
// Reset the selected index so we can assert that `select` was called later
2715+
ngDevMode && setSelectedIndex(-1);
2716+
26512717
templateFn(getRenderFlags(hostView), component);
2718+
26522719
refreshDescendantViews(hostView);
26532720
// Only check view queries again in creation mode if there are static view queries
26542721
if (!creationMode || hostTView.staticViewQueries) {

packages/core/src/render3/state.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {assertDefined} from '../util/assert';
9+
import {assertDefined, assertGreaterThan} from '../util/assert';
1010

1111
import {assertLViewOrUndefined} from './assert';
1212
import {executeHooks} from './hooks';
@@ -340,3 +340,28 @@ export function leaveView(newView: LView): void {
340340
}
341341
enterView(newView, null);
342342
}
343+
344+
let _selectedIndex = -1;
345+
346+
/**
347+
* Gets the most recent index passed to {@link select}
348+
*
349+
* Used with {@link property} instruction (and more in the future) to identify the index in the
350+
* current `LView` to act on.
351+
*/
352+
export function getSelectedIndex() {
353+
ngDevMode &&
354+
assertGreaterThan(
355+
_selectedIndex, -1, 'select() should be called prior to retrieving the selected index');
356+
return _selectedIndex;
357+
}
358+
359+
/**
360+
* Sets the most recent index passed to {@link select}
361+
*
362+
* Used with {@link property} instruction (and more in the future) to identify the index in the
363+
* current `LView` to act on.
364+
*/
365+
export function setSelectedIndex(index: number) {
366+
_selectedIndex = index;
367+
}

packages/core/test/render3/instructions_spec.ts

Lines changed: 94 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {NgForOfContext} from '@angular/common';
1010

1111
import {RenderFlags} from '../../src/render3';
1212
import {defineComponent} from '../../src/render3/definition';
13-
import {bind, element, elementAttribute, elementEnd, elementProperty, elementStart, elementStyleProp, elementStyling, elementStylingApply, elementStylingMap, interpolation1, renderTemplate, template, text, textBinding} from '../../src/render3/instructions/all';
13+
import {bind, element, elementAttribute, elementEnd, elementProperty, elementStart, elementStyleProp, elementStyling, elementStylingApply, elementStylingMap, interpolation1, renderTemplate, template, text, textBinding, select, property} from '../../src/render3/instructions/all';
1414
import {AttributeMarker} from '../../src/render3/interfaces/node';
1515
import {bypassSanitizationTrustHtml, bypassSanitizationTrustResourceUrl, bypassSanitizationTrustScript, bypassSanitizationTrustStyle, bypassSanitizationTrustUrl} from '../../src/sanitization/bypass';
1616
import {defaultStyleSanitizer, sanitizeHtml, sanitizeResourceUrl, sanitizeScript, sanitizeStyle, sanitizeUrl} from '../../src/sanitization/sanitization';
@@ -19,6 +19,7 @@ import {StyleSanitizeFn} from '../../src/sanitization/style_sanitizer';
1919

2020
import {NgForOf} from './common_with_def';
2121
import {ComponentFixture, TemplateFixture} from './render_util';
22+
import {setSelectedIndex, getSelectedIndex} from '@angular/core/src/render3/state';
2223

2324
describe('instructions', () => {
2425
function createAnchor() {
@@ -162,6 +163,98 @@ describe('instructions', () => {
162163
});
163164
});
164165

166+
describe('select', () => {
167+
it('should error in DevMode if index is out of range', () => {
168+
// Only one constant added, meaning only index `0` is valid.
169+
const t = new TemplateFixture(createDiv, () => {}, 1, 0);
170+
expect(() => { t.update(() => { select(-1); }); }).toThrow();
171+
expect(() => { t.update(() => { select(1); }); }).toThrow();
172+
expect(() => { t.update(() => { select(0); }); }).not.toThrow();
173+
});
174+
});
175+
176+
describe('property', () => {
177+
// TODO(benlesh): Replace with TestBed tests once the instruction is being generated.
178+
it('should set properties of the selected element', () => {
179+
// <div [title]="title"></div>
180+
const t = new TemplateFixture(createDiv, () => {}, 1, 1);
181+
t.update(() => {
182+
select(0);
183+
property('title', 'one');
184+
});
185+
expect(t.html).toEqual('<div title="one"></div>');
186+
t.update(() => {
187+
select(0);
188+
property('title', 'two');
189+
});
190+
expect(t.html).toEqual('<div title="two"></div>');
191+
expect(ngDevMode).toHaveProperties({
192+
firstTemplatePass: 1,
193+
tNode: 2, // 1 for div, 1 for host element
194+
tView: 2, // 1 for rootView + 1 for the template view
195+
rendererCreateElement: 1,
196+
rendererSetProperty: 2,
197+
});
198+
});
199+
200+
// TODO(benlesh): Replace with TestBed tests once the instruction is being generated.
201+
it('should chain', () => {
202+
// <div [title]="title" [accesskey]="key"></div>
203+
const t = new TemplateFixture(createDiv, () => {}, 1, 2);
204+
t.update(() => {
205+
select(0);
206+
property('title', 'one')('accessKey', 'A');
207+
});
208+
expect(t.html).toEqual('<div accesskey="A" title="one"></div>');
209+
t.update(() => {
210+
select(0);
211+
property('title', 'two')('accessKey', 'B');
212+
});
213+
expect(t.html).toEqual('<div accesskey="B" title="two"></div>');
214+
expect(ngDevMode).toHaveProperties({
215+
firstTemplatePass: 1,
216+
tNode: 2, // 1 for div, 1 for host element
217+
tView: 2, // 1 for rootView + 1 for the template view
218+
rendererCreateElement: 1,
219+
rendererSetProperty: 4,
220+
});
221+
});
222+
223+
// TODO(benlesh): Replace with TestBed tests once the instruction is being generated.
224+
it('should diff value changes', () => {
225+
// <div [title]="title" [accesskey]="key"></div>
226+
const t = new TemplateFixture(createDiv, () => {}, 1, 2);
227+
t.update(() => {
228+
select(0);
229+
property('title', 'one')('accessKey', 'A');
230+
});
231+
expect(t.html).toEqual('<div accesskey="A" title="one"></div>');
232+
t.update(() => {
233+
select(0);
234+
property('title', 'two')('accessKey', 'A'); // Notice: only changing the title.
235+
});
236+
expect(t.html).toEqual('<div accesskey="A" title="two"></div>');
237+
expect(ngDevMode).toHaveProperties({
238+
firstTemplatePass: 1,
239+
tNode: 2, // 1 for div, 1 for host element
240+
tView: 2, // 1 for rootView + 1 for the template view
241+
rendererCreateElement: 1,
242+
rendererSetProperty: 3,
243+
});
244+
});
245+
246+
it('should error in dev mode if select was not called prior', () => {
247+
const t = new TemplateFixture(createDiv, () => {}, 1, 1);
248+
expect(() => { t.update(() => { property('title', 'test'); }); }).toThrow();
249+
expect(() => {
250+
t.update(() => {
251+
select(0);
252+
property('title', 'test');
253+
});
254+
}).not.toThrow();
255+
});
256+
});
257+
165258
describe('elementProperty', () => {
166259
it('should use sanitizer function when available', () => {
167260
const t = new TemplateFixture(createDiv, () => {}, 1);

0 commit comments

Comments
 (0)