Revert "feat(ivy): add property instruction (angular#29513)"

This reverts commit e4c1c88.

Author: mhevery

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, assertGreaterThan, assertLessThan, assertNotEqual} from '../../util/assert';
+import {assertDataInRange, assertDefined, assertDomNode, assertEqual, 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, getSelectedIndex, increaseElementDepthCount, isCreationMode, leaveView, nextContextImpl, resetComponentState, setActiveHost, setBindingRoot, setCheckNoChangesMode, setCurrentDirectiveDef, setCurrentQueryIndex, setIsParent, setPreviousOrParentTNode, setSelectedIndex} from '../state';
+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 {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,10 +408,6 @@ 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
@@ -457,10 +453,6 @@ 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);
       }
 
@@ -1101,30 +1093,11 @@ export function elementEnd(): void {
 
 
 /**
- * Selects an index of an item to act on and flushes lifecycle hooks up to this point
+ * Flushes all the lifecycle hooks for directives up until (and excluding) that node index
  *
- * 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
-  */
+ * @param index The index of the element in the `LView`
+ */
 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);
 }
@@ -1169,34 +1142,6 @@ 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,
@@ -2703,12 +2648,7 @@ 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) {

packages/core/src/render3/state.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {assertDefined, assertGreaterThan} from '../util/assert';
+import {assertDefined} from '../util/assert';
 
 import {assertLViewOrUndefined} from './assert';
 import {executeHooks} from './hooks';
@@ -340,28 +340,3 @@ 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;
-}

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, select, property} from '../../src/render3/instructions/all';
+import {bind, element, elementAttribute, elementEnd, elementProperty, elementStart, elementStyleProp, elementStyling, elementStylingApply, elementStylingMap, interpolation1, renderTemplate, template, text, textBinding} 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,7 +19,6 @@ 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() {
@@ -163,98 +162,6 @@ 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);