fix(core): Access injected parent values using SelfSkip (#39464)

In ViewEngine, SelfSkip would navigate up the tree to get tokens from
the parent node, skipping the child. This restores that functionality in
Ivy. In ViewEngine, if a special token (e.g. ElementRef) was not found
in the NodeInjector tree, the ModuleInjector was also used to lookup
that token. While special tokens like ElementRef make sense only in a
context of a NodeInjector, we preserved ViewEngine logic for now to
avoid breaking changes.

We identified 4 scenarios related to @SkipSelf and special tokens where
ViewEngine behavior was incorrect and is likely due to bugs. In Ivy this
is implemented to provide a more intuitive API. The list of scenarios
can be found below.

1. When Injector is used in combination with @Host and @SkipSelf on the
first Component within a module and the injector is defined in the
module, ViewEngine will get the injector from the module. In Ivy, it
does not do this and throws instead.

2. When retrieving a @ViewContainerRef while @SkipSelf and @Host are
present, in ViewEngine, it throws an exception. In Ivy it returns the
host ViewContainerRef.

3. When retrieving a @ViewContainerRef on an embedded view and @SkipSelf
is present, in ViewEngine, the ref is null. In Ivy it returns the parent
ViewContainerRef.

4. When utilizing viewProviders and providers, a child component that is
nested within a parent component that has @SkipSelf on a viewProvider
value, if that provider is provided by the parent component's
viewProviders and providers, ViewEngine will return that parent's
viewProviders value, which violates how viewProviders' visibility should
work. In Ivy, it retrieves the value from providers, as it should.

These discrepancies all behave as they should in Ivy and are likely bugs
in ViewEngine.

PR Close #39464

goldens/size-tracking/integration-payloads.json

@@ -12,7 +12,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 1485,
-        "main-es2015": 17210,
+        "main-es2015": 17597,
         "polyfills-es2015": 36709
       }
     }
@@ -21,7 +21,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 1485,
-        "main-es2015": 146698,
+        "main-es2015": 147252,
         "polyfills-es2015": 36964
       }
     }
@@ -30,7 +30,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 1485,
-        "main-es2015": 136062,
+        "main-es2015": 136703,
         "polyfills-es2015": 37641
       }
     }
@@ -66,4 +66,4 @@
       }
     }
   }
-}
+}

packages/core/src/render3/di.ts

@@ -27,7 +27,7 @@ import {AttributeMarker, TContainerNode, TDirectiveHostNode, TElementContainerNo
 import {isComponentDef, isComponentHost} from './interfaces/type_checks';
 import {DECLARATION_COMPONENT_VIEW, DECLARATION_VIEW, INJECTOR, LView, T_HOST, TData, TVIEW, TView, TViewType} from './interfaces/view';
 import {assertTNodeType} from './node_assert';
-import {enterDI, leaveDI} from './state';
+import {enterDI, getCurrentTNode, getLView, leaveDI} from './state';
 import {isNameOnlyAttributeMarker} from './util/attrs_utils';
 import {getParentInjectorIndex, getParentInjectorView, hasParentInjector} from './util/injector_utils';
 import {stringifyForError} from './util/misc_utils';
@@ -345,6 +345,51 @@ export function injectAttributeImpl(tNode: TNode, attrNameToInject: string): str
 }
 
 
+function notFoundValueOrThrow<T>(
+    notFoundValue: T|null, token: Type<T>|InjectionToken<T>, flags: InjectFlags): T|null {
+  if (flags & InjectFlags.Optional) {
+    return notFoundValue;
+  } else {
+    throwProviderNotFoundError(token, 'NodeInjector');
+  }
+}
+
+/**
+ * Returns the value associated to the given token from the ModuleInjector or throws exception
+ *
+ * @param lView The `LView` that contains the `tNode`
+ * @param token The token to look for
+ * @param flags Injection flags
+ * @param notFoundValue The value to return when the injection flags is `InjectFlags.Optional`
+ * @returns the value from the injector or throws an exception
+ */
+function lookupTokenUsingModuleInjector<T>(
+    lView: LView, token: Type<T>|InjectionToken<T>, flags: InjectFlags, notFoundValue?: any): T|
+    null {
+  if (flags & InjectFlags.Optional && notFoundValue === undefined) {
+    // This must be set or the NullInjector will throw for optional deps
+    notFoundValue = null;
+  }
+
+  if ((flags & (InjectFlags.Self | InjectFlags.Host)) === 0) {
+    const moduleInjector = lView[INJECTOR];
+    // switch to `injectInjectorOnly` implementation for module injector, since module injector
+    // should not have access to Component/Directive DI scope (that may happen through
+    // `directiveInject` implementation)
+    const previousInjectImplementation = setInjectImplementation(undefined);
+    try {
+      if (moduleInjector) {
+        return moduleInjector.get(token, notFoundValue, flags & InjectFlags.Optional);
+      } else {
+        return injectRootLimpMode(token, notFoundValue, flags & InjectFlags.Optional);
+      }
+    } finally {
+      setInjectImplementation(previousInjectImplementation);
+    }
+  }
+  return notFoundValueOrThrow<T>(notFoundValue, token, flags);
+}
+
 /**
  * Returns the value associated to the given token from the NodeInjectors => ModuleInjector.
  *
@@ -352,8 +397,7 @@ export function injectAttributeImpl(tNode: TNode, attrNameToInject: string): str
  * the module injector tree.
  *
  * This function patches `token` with `__NG_ELEMENT_ID__` which contains the id for the bloom
- * filter. Negative values are reserved for special objects.
+ * filter. `-1` is reserved for injecting `Injector` (implemented by `NodeInjector`)
- *   - `-1` is reserved for injecting `Injector` (implemented by `NodeInjector`)
  *
  * @param tNode The Node where the search for the injector should start
  * @param lView The `LView` that contains the `tNode`
@@ -370,7 +414,10 @@ export function getOrCreateInjectable<T>(
     // If the ID stored here is a function, this is a special object like ElementRef or TemplateRef
     // so just call the factory function to create it.
     if (typeof bloomHash === 'function') {
-      enterDI(lView, tNode);
+      if (!enterDI(lView, tNode, flags)) {
+        // Failed to enter DI use module injector instead.
+        return lookupTokenUsingModuleInjector<T>(lView, token, flags, notFoundValue);
+      }
       try {
         const value = bloomHash();
         if (value == null && !(flags & InjectFlags.Optional)) {
@@ -381,10 +428,30 @@ export function getOrCreateInjectable<T>(
       } finally {
         leaveDI();
       }
-    } else if (typeof bloomHash == 'number') {
+    } else if (typeof bloomHash === 'number') {
+      // This is a value used to identify __NG_ELEMENT_ID__
+      // `-1` is a special value used to identify `Injector` types in NodeInjector
+      // This is a workaround for the fact that if the `Injector.__NG_ELEMENT_ID__`
+      // would have a factory function (such as `ElementRef`) it would cause Ivy
+      // to be pulled into the ViewEngine, because they both share `Injector` type.
+      // This should be refactored to follow `ElementRef` pattern once ViewEngine is
+      // removed
       if (bloomHash === -1) {
-        // `-1` is a special value used to identify `Injector` types.
+        if (!enterDI(lView, tNode, flags)) {
-        return new NodeInjector(tNode, lView) as any;
+          // Failed to enter DI, try module injector instead. If a token is injected with the @Host
+          // flag, the module injector is not searched for that token in Ivy.
+          return (flags & InjectFlags.Host) ?
+              notFoundValueOrThrow<T>(notFoundValue, token, flags) :
+              lookupTokenUsingModuleInjector<T>(lView, token, flags, notFoundValue);
+        }
+        try {
+          // Retrieving current `TNode` and `LView` from the state (rather than using `tNode` and
+          // `lView`), because entering DI (by calling `enterDI`) may cause these values to change
+          // (in case `@SkipSelf` flag is present).
+          return new NodeInjector(getCurrentTNode()! as TDirectiveHostNode, getLView()) as any;
+        } finally {
+          leaveDI();
+        }
       }
       // If the token has a bloom hash, then it is a token which could be in NodeInjector.
 
@@ -453,32 +520,7 @@ export function getOrCreateInjectable<T>(
     }
   }
 
-  if (flags & InjectFlags.Optional && notFoundValue === undefined) {
+  return lookupTokenUsingModuleInjector<T>(lView, token, flags, notFoundValue);
-    // This must be set or the NullInjector will throw for optional deps
-    notFoundValue = null;
-  }
-
-  if ((flags & (InjectFlags.Self | InjectFlags.Host)) === 0) {
-    const moduleInjector = lView[INJECTOR];
-    // switch to `injectInjectorOnly` implementation for module injector, since module injector
-    // should not have access to Component/Directive DI scope (that may happen through
-    // `directiveInject` implementation)
-    const previousInjectImplementation = setInjectImplementation(undefined);
-    try {
-      if (moduleInjector) {
-        return moduleInjector.get(token, notFoundValue, flags & InjectFlags.Optional);
-      } else {
-        return injectRootLimpMode(token, notFoundValue, flags & InjectFlags.Optional);
-      }
-    } finally {
-      setInjectImplementation(previousInjectImplementation);
-    }
-  }
-  if (flags & InjectFlags.Optional) {
-    return notFoundValue;
-  } else {
-    throwProviderNotFoundError(token, 'NodeInjector');
-  }
 }
 
 const NOT_FOUND = {};
@@ -582,7 +624,11 @@ export function getNodeInjectable(
     factory.resolving = true;
     const previousInjectImplementation =
         factory.injectImpl ? setInjectImplementation(factory.injectImpl) : null;
-    enterDI(lView, tNode);
+    const success = enterDI(lView, tNode, InjectFlags.Default);
+    ngDevMode &&
+        assertEqual(
+            success, true,
+            'Because flags do not contain \`SkipSelf\' we expect this to always succeed.');
     try {
       value = lView[index] = factory.factory(undefined, tData, lView, tNode);
       // This code path is hit for both directives and providers.

packages/core/src/render3/state.ts

@@ -6,13 +6,13 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {InjectFlags} from '../di/interface/injector';
 import {assertDefined, assertEqual, assertGreaterThanOrEqual, assertLessThan, assertNotEqual} from '../util/assert';
-import {assertLViewOrUndefined, assertTNodeForTView} from './assert';
+import {assertLViewOrUndefined, assertTNodeForLView, assertTNodeForTView} from './assert';
 import {DirectiveDef} from './interfaces/definition';
 import {TNode, TNodeType} from './interfaces/node';
-import {CONTEXT, DECLARATION_VIEW, HEADER_OFFSET, LView, OpaqueViewState, TData, TVIEW, TView} from './interfaces/view';
+import {CONTEXT, DECLARATION_VIEW, HEADER_OFFSET, LView, OpaqueViewState, T_HOST, TData, TVIEW, TView, TViewType} from './interfaces/view';
 import {MATH_ML_NAMESPACE, SVG_NAMESPACE} from './namespaces';
-import {assertTNodeType} from './node_assert';
 import {getTNode} from './util/view_utils';
 
 
@@ -434,16 +434,89 @@ export function setCurrentQueryIndex(value: number): void {
 }
 
 /**
- * This is a light weight version of the `enterView` which is needed by the DI system.
+ * Returns a `TNode` of the location where the current `LView` is declared at.
- * @param newView
+ *
- * @param tNode
+ * @param lView an `LView` that we want to find parent `TNode` for.
  */
-export function enterDI(newView: LView, tNode: TNode) {
+function getDeclarationTNode(lView: LView): TNode|null {
-  ngDevMode && assertLViewOrUndefined(newView);
+  const tView = lView[TVIEW];
-  const newLFrame = allocLFrame();
+
-  instructionState.lFrame = newLFrame;
+  // Return the declaration parent for embedded views
-  newLFrame.currentTNode = tNode!;
+  if (tView.type === TViewType.Embedded) {
-  newLFrame.lView = newView;
+    ngDevMode && assertDefined(tView.declTNode, 'Embedded TNodes should have declaration parents.');
+    return tView.declTNode;
+  }
+
+  // Components don't have `TView.declTNode` because each instance of component could be
+  // inserted in different location, hence `TView.declTNode` is meaningless.
+  // Falling back to `T_HOST` in case we cross component boundary.
+  if (tView.type === TViewType.Component) {
+    return lView[T_HOST];
+  }
+
+  // Remaining TNode type is `TViewType.Root` which doesn't have a parent TNode.
+  return null;
+}
+
+/**
+ * This is a light weight version of the `enterView` which is needed by the DI system.
+ *
+ * @param lView `LView` location of the DI context.
+ * @param tNode `TNode` for DI context
+ * @param flags DI context flags. if `SkipSelf` flag is set than we walk up the declaration
+ *     tree from `tNode`  until we find parent declared `TElementNode`.
+ * @returns `true` if we have successfully entered DI associated with `tNode` (or with declared
+ *     `TNode` if `flags` has  `SkipSelf`). Failing to enter DI implies that no associated
+ *     `NodeInjector` can be found and we should instead use `ModuleInjector`.
+ *     - If `true` than this call must be fallowed by `leaveDI`
+ *     - If `false` than this call failed and we should NOT call `leaveDI`
+ */
+export function enterDI(lView: LView, tNode: TNode, flags: InjectFlags) {
+  ngDevMode && assertLViewOrUndefined(lView);
+
+  if (flags & InjectFlags.SkipSelf) {
+    ngDevMode && assertTNodeForTView(tNode, lView[TVIEW]);
+
+    let parentTNode = tNode as TNode | null;
+    let parentLView = lView;
+
+    while (true) {
+      ngDevMode && assertDefined(parentTNode, 'Parent TNode should be defined');
+      parentTNode = parentTNode!.parent as TNode | null;
+      if (parentTNode === null && !(flags & InjectFlags.Host)) {
+        parentTNode = getDeclarationTNode(parentLView);
+        if (parentTNode === null) break;
+
+        // In this case, a parent exists and is definitely an element. So it will definitely
+        // have an existing lView as the declaration view, which is why we can assume it's defined.
+        ngDevMode && assertDefined(parentLView, 'Parent LView should be defined');
+        parentLView = parentLView[DECLARATION_VIEW]!;
+
+        // In Ivy there are Comment nodes that correspond to ngIf and NgFor embedded directives
+        // We want to skip those and look only at Elements and ElementContainers to ensure
+        // we're looking at true parent nodes, and not content or other types.
+        if (parentTNode.type & (TNodeType.Element | TNodeType.ElementContainer)) {
+          break;
+        }
+      } else {
+        break;
+      }
+    }
+    if (parentTNode === null) {
+      // If we failed to find a parent TNode this means that we should use module injector.
+      return false;
+    } else {
+      tNode = parentTNode;
+      lView = parentLView;
+    }
+  }
+
+  ngDevMode && assertTNodeForLView(tNode, lView);
+  const lFrame = instructionState.lFrame = allocLFrame();
+  lFrame.currentTNode = tNode;
+  lFrame.lView = lView;
+
+  return true;
 }
 
 /**

packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json

@@ -14,6 +14,9 @@
   {
     "name": "EMPTY_OBJ"
   },
+  {
+    "name": "InjectFlags"
+  },
   {
     "name": "Module"
   },
@@ -173,6 +176,9 @@
   {
     "name": "getCurrentTNodePlaceholderOk"
   },
+  {
+    "name": "getDeclarationTNode"
+  },
   {
     "name": "getFirstLContainer"
   },

packages/core/test/bundling/forms/bundle.golden_symbols.json

@@ -983,6 +983,9 @@
   {
     "name": "getDebugContext"
   },
+  {
+    "name": "getDeclarationTNode"
+  },
   {
     "name": "getFactoryDef"
   },
@@ -1286,6 +1289,9 @@
   {
     "name": "localeEn"
   },
+  {
+    "name": "lookupTokenUsingModuleInjector"
+  },
   {
     "name": "makeParamDecorator"
   },
@@ -1379,6 +1385,9 @@
   {
     "name": "normalizeValidators"
   },
+  {
+    "name": "notFoundValueOrThrow"
+  },
   {
     "name": "observable"
   },

packages/core/test/bundling/hello_world/bundle.golden_symbols.json

@@ -11,6 +11,9 @@
   {
     "name": "EMPTY_OBJ"
   },
+  {
+    "name": "InjectFlags"
+  },
   {
     "name": "NG_COMP_DEF"
   },
@@ -128,6 +131,9 @@
   {
     "name": "getCurrentTNodePlaceholderOk"
   },
+  {
+    "name": "getDeclarationTNode"
+  },
   {
     "name": "getFirstLContainer"
   },

packages/core/test/bundling/router/bundle.golden_symbols.json

@@ -1292,6 +1292,9 @@
   {
     "name": "getDebugContext"
   },
+  {
+    "name": "getDeclarationTNode"
+  },
   {
     "name": "getFactoryDef"
   },
@@ -1607,6 +1610,9 @@
   {
     "name": "locateDirectiveOrProvider"
   },
+  {
+    "name": "lookupTokenUsingModuleInjector"
+  },
   {
     "name": "makeParamDecorator"
   },
@@ -1703,6 +1709,9 @@
   {
     "name": "normalizeQueryParams"
   },
+  {
+    "name": "notFoundValueOrThrow"
+  },
   {
     "name": "observable"
   },

packages/core/test/bundling/todo/bundle.golden_symbols.json

@@ -353,6 +353,9 @@
   {
     "name": "getDebugContext"
   },
+  {
+    "name": "getDeclarationTNode"
+  },
   {
     "name": "getFirstLContainer"
   },
@@ -542,6 +545,9 @@
   {
     "name": "leaveViewLight"
   },
+  {
+    "name": "lookupTokenUsingModuleInjector"
+  },
   {
     "name": "makeParamDecorator"
   },
@@ -587,6 +593,9 @@
   {
     "name": "noSideEffects"
   },
+  {
+    "name": "notFoundValueOrThrow"
+  },
   {
     "name": "readPatchedData"
   },