refactor(core): remove the need for `ɵɵinjectPipeChangeDetectorRef()` (#41231)

This instruction was created to work around a problem with injecting a
`ChangeDetectorRef` into a pipe. See #31438. This fix required special
metadata for when the thing being injected was a `ChangeDetectorRef`.

Now this is handled by adding a flag `InjectorFlags.ForPipe` to the
`ɵɵdirectiveInject()` call, which avoids the need to special test_cases
`ChangeDetectorRef` in the generated code.

PR Close #41231
This commit is contained in:
Pete Bacon Darwin 2021-03-19 07:18:33 +00:00 committed by Alex Rickabaugh
parent cf4f74aad0
commit 857dfaa1e7
30 changed files with 50 additions and 98 deletions

View File

@ -694,9 +694,6 @@ export declare interface ɵɵInjectableDef<T> {
/** @codeGenApi */
export declare function ɵɵinjectAttribute(attrNameToInject: string): string | null;
/** @codeGenApi */
export declare function ɵɵinjectPipeChangeDetectorRef(flags?: InjectFlags): ChangeDetectorRef | null;
export declare const PACKAGE_ROOT_URL: InjectionToken<string>;
export declare interface Pipe {

View File

@ -12,8 +12,8 @@
"master": {
"uncompressed": {
"runtime-es2015": 3033,
"main-es2015": 452165,
"polyfills-es2015": 52493
"main-es2015": 451600,
"polyfills-es2015": 52215
}
}
},
@ -26,4 +26,4 @@
}
}
}
}
}

View File

@ -823,7 +823,7 @@ runInEachFileSystem(() => {
expect(jsContents)
.toContain(
`TestClass.ɵfac = function TestClass_Factory(t) { ` +
`return new (t || TestClass)(ɵngcc0.ɵɵinjectPipeChangeDetectorRef()); };`);
`return new (t || TestClass)(ɵngcc0.ɵɵdirectiveInject(ɵngcc0.ChangeDetectorRef, 16)); };`);
});
it('should use the correct type name in typings files when an export has a different name in source files',

View File

@ -86,10 +86,6 @@ export function getConstructorDependencies(
}
});
if (token instanceof ExternalExpr && token.value.name === 'ChangeDetectorRef' &&
token.value.moduleName === '@angular/core') {
resolved = R3ResolvedDependencyType.ChangeDetectorRef;
}
if (token === null) {
if (param.typeValueReference.kind !== TypeValueReferenceKind.UNAVAILABLE) {
throw new Error(

View File

@ -156,7 +156,7 @@ export class MyPipe {
return value;
}
}
MyPipe.ɵfac = i0.ɵɵngDeclareFactory({ version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyPipe, deps: [{ token: i0.ChangeDetectorRef, resolved: i0.ɵɵResolvedDependencyType.ChangeDetectorRef }], target: i0.ɵɵFactoryTarget.Pipe });
MyPipe.ɵfac = i0.ɵɵngDeclareFactory({ version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyPipe, deps: [{ token: i0.ChangeDetectorRef, resolved: i0.ɵɵResolvedDependencyType.Token }], target: i0.ɵɵFactoryTarget.Pipe });
MyPipe.ɵpipe = i0.ɵɵngDeclarePipe({ version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyPipe, name: "myPipe" });
(function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(MyPipe, [{
type: Pipe,
@ -168,7 +168,7 @@ export class MyOtherPipe {
return value;
}
}
MyOtherPipe.ɵfac = i0.ɵɵngDeclareFactory({ version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyOtherPipe, deps: [{ token: i0.ChangeDetectorRef, resolved: i0.ɵɵResolvedDependencyType.ChangeDetectorRef, optional: true }], target: i0.ɵɵFactoryTarget.Pipe });
MyOtherPipe.ɵfac = i0.ɵɵngDeclareFactory({ version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyOtherPipe, deps: [{ token: i0.ChangeDetectorRef, resolved: i0.ɵɵResolvedDependencyType.Token, optional: true }], target: i0.ɵɵFactoryTarget.Pipe });
MyOtherPipe.ɵpipe = i0.ɵɵngDeclarePipe({ version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyOtherPipe, name: "myOtherPipe" });
(function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(MyOtherPipe, [{
type: Pipe,

View File

@ -1 +1 @@
MyOtherPipe.ɵfac = function MyOtherPipe_Factory(t) { return new (t || MyOtherPipe)($r3$.ɵɵinjectPipeChangeDetectorRef(8)); };
MyOtherPipe.ɵfac = function MyOtherPipe_Factory(t) { return new (t || MyOtherPipe)($i0$.ɵɵdirectiveInject($i0$.ChangeDetectorRef, 24)); };

View File

@ -1 +1 @@
MyPipe.ɵfac = function MyPipe_Factory(t) { return new (t || MyPipe)($r3$.ɵɵinjectPipeChangeDetectorRef()); };
MyPipe.ɵfac = function MyPipe_Factory(t) { return new (t || MyPipe)($i0$.ɵɵdirectiveInject($i0$.ChangeDetectorRef, 16)); };

View File

@ -206,7 +206,7 @@ export class MyComponent {
this.cdr = cdr;
}
}
MyComponent.ɵfac = i0.ɵɵngDeclareFactory({ version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, deps: [{ token: i0.ElementRef, resolved: i0.ɵɵResolvedDependencyType.Token }, { token: i0.ViewContainerRef, resolved: i0.ɵɵResolvedDependencyType.Token }, { token: i0.ChangeDetectorRef, resolved: i0.ɵɵResolvedDependencyType.ChangeDetectorRef }], target: i0.ɵɵFactoryTarget.Component });
MyComponent.ɵfac = i0.ɵɵngDeclareFactory({ version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, deps: [{ token: i0.ElementRef, resolved: i0.ɵɵResolvedDependencyType.Token }, { token: i0.ViewContainerRef, resolved: i0.ɵɵResolvedDependencyType.Token }, { token: i0.ChangeDetectorRef, resolved: i0.ɵɵResolvedDependencyType.Token }], target: i0.ɵɵFactoryTarget.Component });
MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ version: "0.0.0-PLACEHOLDER", type: MyComponent, selector: "my-component", ngImport: i0, template: '', isInline: true });
(function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(MyComponent, [{
type: Component,

View File

@ -1,4 +1,4 @@
// NOTE The prov definition must be last so MyOtherPipe.fac is defined
MyOtherPipe.ɵfac = function MyOtherPipe_Factory(t) { return new (t || MyOtherPipe)($r3$.ɵɵdirectiveInject(Service)); };
MyOtherPipe.ɵfac = function MyOtherPipe_Factory(t) { return new (t || MyOtherPipe)($r3$.ɵɵdirectiveInject(Service, 16)); };
MyOtherPipe.ɵpipe = /*@__PURE__*/ i0.ɵɵdefinePipe({ name: "myOtherPipe", type: MyOtherPipe, pure: true });
MyOtherPipe.ɵprov = /*@__PURE__*/ i0.ɵɵdefineInjectable({ token: MyOtherPipe, factory: MyOtherPipe.ɵfac });

View File

@ -1,4 +1,4 @@
// NOTE The prov definition must be last so MyPipe.fac is defined
MyPipe.ɵfac = function MyPipe_Factory(t) { return new (t || MyPipe)(i0.ɵɵdirectiveInject(Service)); };
MyPipe.ɵfac = function MyPipe_Factory(t) { return new (t || MyPipe)(i0.ɵɵdirectiveInject(Service, 16)); };
MyPipe.ɵpipe = /*@__PURE__*/ i0.ɵɵdefinePipe({ name: "myPipe", type: MyPipe, pure: true });
MyPipe.ɵprov = /*@__PURE__*/ i0.ɵɵdefineInjectable({ token: MyPipe, factory: MyPipe.ɵfac });

View File

@ -2294,7 +2294,7 @@ describe('compiler compliance', () => {
`;
const MyPipeFactory = `
MyPipe.ɵfac = function MyPipe_Factory(t) { return new (t || MyPipe)($r3$.ɵɵinjectPipeChangeDetectorRef()); };
MyPipe.ɵfac = function MyPipe_Factory(t) { return new (t || MyPipe)($i0$.ɵɵdirectiveInject($i0$.ChangeDetectorRef, 16)); };
`;
const MyOtherPipeDefinition = `
@ -2305,7 +2305,7 @@ describe('compiler compliance', () => {
});`;
const MyOtherPipeFactory = `
MyOtherPipe.ɵfac = function MyOtherPipe_Factory(t) { return new (t || MyOtherPipe)($r3$.ɵɵinjectPipeChangeDetectorRef(8)); };
MyOtherPipe.ɵfac = function MyOtherPipe_Factory(t) { return new (t || MyOtherPipe)($i0$.ɵɵdirectiveInject($i0$.ChangeDetectorRef, 24)); };
`;
const result = compile(files, angularFiles);

View File

@ -372,14 +372,14 @@ describe('compiler compliance: dependency injection', () => {
// The prov definition must be last so MyPipe.fac is defined
const MyPipeDefs = `
MyPipe.ɵfac = function MyPipe_Factory(t) { return new (t || MyPipe)(i0.ɵɵdirectiveInject(Service)); };
MyPipe.ɵfac = function MyPipe_Factory(t) { return new (t || MyPipe)(i0.ɵɵdirectiveInject(Service, 16)); };
MyPipe.ɵpipe = /*@__PURE__*/ i0.ɵɵdefinePipe({ name: "myPipe", type: MyPipe, pure: true });
MyPipe.ɵprov = /*@__PURE__*/ i0.ɵɵdefineInjectable({ token: MyPipe, factory: MyPipe.ɵfac });
`;
// The prov definition must be last so MyOtherPipe.fac is defined
const MyOtherPipeDefs = `
MyOtherPipe.ɵfac = function MyOtherPipe_Factory(t) { return new (t || MyOtherPipe)($r3$.ɵɵdirectiveInject(Service)); };
MyOtherPipe.ɵfac = function MyOtherPipe_Factory(t) { return new (t || MyOtherPipe)($r3$.ɵɵdirectiveInject(Service, 16)); };
MyOtherPipe.ɵpipe = /*@__PURE__*/ i0.ɵɵdefinePipe({ name: "myOtherPipe", type: MyOtherPipe, pure: true });
MyOtherPipe.ɵprov = /*@__PURE__*/ i0.ɵɵdefineInjectable({ token: MyOtherPipe, factory: MyOtherPipe.ɵfac });
`;

View File

@ -1565,7 +1565,7 @@ function allTests(os: string) {
env.driveMain();
const jsContents = env.getContents('test.js');
expect(jsContents).toContain('return new (t || TestPipe)(i0.ɵɵdirectiveInject(Dep));');
expect(jsContents).toContain('return new (t || TestPipe)(i0.ɵɵdirectiveInject(Dep, 16));');
});
it('should compile Pipes with generic types', () => {

View File

@ -86,8 +86,7 @@ export type Provider = any;
export enum R3ResolvedDependencyType {
Token = 0,
Attribute = 1,
ChangeDetectorRef = 2,
Invalid = 3,
Invalid = 2,
}
export enum R3FactoryTarget {

View File

@ -272,6 +272,11 @@ export const enum InjectFlags {
SkipSelf = 1 << 2,
/** Inject `defaultValue` instead if token not found. */
Optional = 1 << 3,
/**
* This token is being injected into a pipe.
* @internal
*/
ForPipe = 1 << 4,
}
export const enum ArgumentType {

View File

@ -432,13 +432,8 @@ export enum R3ResolvedDependencyType {
*/
Attribute = 1,
/**
* Injecting the `ChangeDetectorRef` token. Needs special handling when injected into a pipe.
*/
ChangeDetectorRef = 2,
/**
* An invalid dependency (no token could be determined). An error should be thrown at runtime.
*/
Invalid = 3,
Invalid = 2,
}

View File

@ -109,15 +109,10 @@ export enum R3ResolvedDependencyType {
*/
Attribute = 1,
/**
* Injecting the `ChangeDetectorRef` token. Needs special handling when injected into a pipe.
*/
ChangeDetectorRef = 2,
/**
* An invalid dependency (no token could be determined). An error should be thrown at runtime.
*/
Invalid = 3,
Invalid = 2,
}
/**
@ -270,16 +265,14 @@ function injectDependencies(deps: R3DependencyMetadata[], target: R3FactoryTarge
function compileInjectDependency(
dep: R3DependencyMetadata, target: R3FactoryTarget, index: number): o.Expression {
const isPipe = target === R3FactoryTarget.Pipe;
// Interpret the dependency according to its resolved type.
switch (dep.resolved) {
case R3ResolvedDependencyType.Token:
case R3ResolvedDependencyType.ChangeDetectorRef:
// Build up the injection flags according to the metadata.
const flags = InjectFlags.Default | (dep.self ? InjectFlags.Self : 0) |
(dep.skipSelf ? InjectFlags.SkipSelf : 0) | (dep.host ? InjectFlags.Host : 0) |
(dep.optional ? InjectFlags.Optional : 0);
(dep.optional ? InjectFlags.Optional : 0) |
(target === R3FactoryTarget.Pipe ? InjectFlags.ForPipe : 0);
// If this dependency is optional or otherwise has non-default flags, then additional
// parameters describing how to inject the dependency must be passed to the inject function
@ -287,11 +280,6 @@ function compileInjectDependency(
let flagsParam: o.LiteralExpr|null =
(flags !== InjectFlags.Default || dep.optional) ? o.literal(flags) : null;
// We have a separate instruction for injecting ChangeDetectorRef into a pipe.
if (isPipe && dep.resolved === R3ResolvedDependencyType.ChangeDetectorRef) {
return o.importExpr(R3.injectPipeChangeDetectorRef).callFn(flagsParam ? [flagsParam] : []);
}
// Build up the arguments to the injectFn call.
const injectArgs = [dep.token];
if (flagsParam) {

View File

@ -219,9 +219,6 @@ export class Identifiers {
static injectAttribute: o.ExternalReference = {name: 'ɵɵinjectAttribute', moduleName: CORE};
static injectPipeChangeDetectorRef:
o.ExternalReference = {name: 'ɵɵinjectPipeChangeDetectorRef', moduleName: CORE};
static directiveInject: o.ExternalReference = {name: 'ɵɵdirectiveInject', moduleName: CORE};
static invalidFactory: o.ExternalReference = {name: 'ɵɵinvalidFactory', moduleName: CORE};
static invalidFactoryDep: o.ExternalReference = {name: 'ɵɵinvalidFactoryDep', moduleName: CORE};

View File

@ -153,7 +153,8 @@ import * as core from '@angular/core';
expectToBe(compilerCore.InjectFlags.Default, core.InjectFlags.Default);
expectToBe(compilerCore.InjectFlags.SkipSelf, core.InjectFlags.SkipSelf);
expectToBe(compilerCore.InjectFlags.Self, core.InjectFlags.Self);
expectToBe(compilerCore.InjectFlags.Host, core.InjectFlags.Host);
expectToBe(compilerCore.InjectFlags.Optional, core.InjectFlags.Optional);
expectToBe(compilerCore.ArgumentType.Inline, core.ɵArgumentType.Inline);
expectToBe(compilerCore.ArgumentType.Dynamic, core.ɵArgumentType.Dynamic);

View File

@ -6,6 +6,8 @@
* found in the LICENSE file at https://angular.io/license
*/
import {InjectFlags} from '../di';
import {InternalInjectFlags} from '../di/interface/injector';
import {TNode, TNodeType} from '../render3/interfaces/node';
import {isComponentHost} from '../render3/interfaces/type_checks';
import {DECLARATION_COMPONENT_VIEW, LView} from '../render3/interfaces/view';
@ -125,22 +127,17 @@ export abstract class ChangeDetectorRef {
* @internal
* @nocollapse
*/
static __NG_ELEMENT_ID__: () => ChangeDetectorRef = SWITCH_CHANGE_DETECTOR_REF_FACTORY;
/**
* This marker is need so that the JIT compiler can correctly identify this class as special.
*
* @internal
* @nocollapse
*/
static __ChangeDetectorRef__ = true;
static __NG_ELEMENT_ID__:
(flags: InjectFlags) => ChangeDetectorRef = SWITCH_CHANGE_DETECTOR_REF_FACTORY;
}
/** Returns a ChangeDetectorRef (a.k.a. a ViewRef) */
export function injectChangeDetectorRef(isPipe = false): ChangeDetectorRef {
return createViewRef(getCurrentTNode()!, getLView(), isPipe);
export function injectChangeDetectorRef(flags: InjectFlags): ChangeDetectorRef {
return createViewRef(
getCurrentTNode()!, getLView(),
(flags & InternalInjectFlags.ForPipe) === InternalInjectFlags.ForPipe);
}
/**
@ -152,10 +149,7 @@ export function injectChangeDetectorRef(isPipe = false): ChangeDetectorRef {
* @returns The ChangeDetectorRef to use
*/
function createViewRef(tNode: TNode, lView: LView, isPipe: boolean): ChangeDetectorRef {
// `isComponentView` will be true for Component and Directives (but not for Pipes).
// See https://github.com/angular/angular/pull/33072 for proper fix
const isComponentView = !isPipe && isComponentHost(tNode);
if (isComponentView) {
if (isComponentHost(tNode) && !isPipe) {
// The LView represents the location where the component is declared.
// Instead we want the LView for the component View and so we need to look it up.
const componentView = getComponentLViewByIndex(tNode.index, lView); // look down
@ -167,4 +161,4 @@ function createViewRef(tNode: TNode, lView: LView, isPipe: boolean): ChangeDetec
return new R3_ViewRef(hostComponentView, lView);
}
return null!;
}
}

View File

@ -86,8 +86,7 @@ export type Provider = any;
export enum R3ResolvedDependencyType {
Token = 0,
Attribute = 1,
ChangeDetectorRef = 2,
Invalid = 3,
Invalid = 2,
}
export enum R3FactoryTarget {

View File

@ -154,7 +154,6 @@ export {
ɵɵInheritDefinitionFeature,
ɵɵinjectAttribute,
ɵɵInjectorDeclaration,
ɵɵinjectPipeChangeDetectorRef,
ɵɵinvalidFactory,
ɵɵlistener,
ɵɵloadQuery,

View File

@ -70,4 +70,9 @@ export const enum InternalInjectFlags {
/** Inject `defaultValue` instead if token not found. */
Optional = 0b1000,
}
/**
* This token is being injected into a pipe.
*/
ForPipe = 0b10000,
}

View File

@ -68,9 +68,6 @@ function reflectDependency(compiler: CompilerFacade, dep: any|any[]): R3Dependen
}
meta.token = param.attributeName;
meta.resolved = compiler.R3ResolvedDependencyType.Attribute;
} else if (param.__ChangeDetectorRef__ === true) {
meta.token = param;
meta.resolved = compiler.R3ResolvedDependencyType.ChangeDetectorRef;
} else {
setTokenAndResolvedType(param);
}

View File

@ -421,7 +421,7 @@ export function getOrCreateInjectable<T>(
lookupTokenUsingModuleInjector<T>(lView, token, flags, notFoundValue);
}
try {
const value = bloomHash();
const value = bloomHash(flags);
if (value == null && !(flags & InjectFlags.Optional)) {
throwProviderNotFoundError(token);
} else {

View File

@ -172,7 +172,7 @@ export {
} from './state';
export {NO_CHANGE} from './tokens';
export { ɵɵresolveBody, ɵɵresolveDocument,ɵɵresolveWindow} from './util/misc_utils';
export { ɵɵinjectPipeChangeDetectorRef,ɵɵtemplateRefExtractor} from './view_engine_compatibility_prebound';
export { ɵɵtemplateRefExtractor} from './view_engine_compatibility_prebound';
// clang-format on
export {

View File

@ -42,7 +42,6 @@ export const angularCoreEnv: {[name: string]: Function} =
'ɵɵinjectAttribute': r3.ɵɵinjectAttribute,
'ɵɵinvalidFactory': r3.ɵɵinvalidFactory,
'ɵɵinvalidFactoryDep': ɵɵinvalidFactoryDep,
'ɵɵinjectPipeChangeDetectorRef': r3.ɵɵinjectPipeChangeDetectorRef,
'ɵɵtemplateRefExtractor': r3.ɵɵtemplateRefExtractor,
'ɵɵNgOnChangesFeature': r3.ɵɵNgOnChangesFeature,
'ɵɵProvidersFeature': r3.ɵɵProvidersFeature,

View File

@ -24,18 +24,3 @@ import {LView} from './interfaces/view';
export function ɵɵtemplateRefExtractor(tNode: TNode, lView: LView): TemplateRef<any>|null {
return createTemplateRef(tNode, lView);
}
/**
* Returns the appropriate `ChangeDetectorRef` for a pipe.
*
* @codeGenApi
*/
export function ɵɵinjectPipeChangeDetectorRef(flags = InjectFlags.Default): ChangeDetectorRef|null {
const value = injectChangeDetectorRef(true);
if (value == null && !(flags & InjectFlags.Optional)) {
throwProviderNotFoundError('ChangeDetectorRef');
} else {
return value;
}
}

View File

@ -8,6 +8,7 @@
import {RendererStyleFlags2, RendererType2} from '@angular/core';
import {ChangeDetectorRef} from '@angular/core/src/change_detection/change_detector_ref';
import {InjectFlags} from '@angular/core/src/di';
import {Provider} from '@angular/core/src/di/interface/provider';
import {ElementRef} from '@angular/core/src/linker/element_ref';
import {TemplateRef} from '@angular/core/src/linker/template_ref';
@ -459,7 +460,8 @@ export function enableIvyInjectableFactories() {
(ElementRef as any)[NG_ELEMENT_ID] = () => R3_ELEMENT_REF_FACTORY();
(TemplateRef as any)[NG_ELEMENT_ID] = () => R3_TEMPLATE_REF_FACTORY();
(ViewContainerRef as any)[NG_ELEMENT_ID] = () => R3_VIEW_CONTAINER_REF_FACTORY();
(ChangeDetectorRef as any)[NG_ELEMENT_ID] = () => R3_CHANGE_DETECTOR_REF_FACTORY();
(ChangeDetectorRef as any)[NG_ELEMENT_ID] = (flags: InjectFlags) =>
R3_CHANGE_DETECTOR_REF_FACTORY(flags);
(Renderer2 as any)[NG_ELEMENT_ID] = () => R3_RENDERER2_FACTORY();
}

View File

@ -627,12 +627,6 @@ describe('TestBed', () => {
});
it('should handle overrides for a provider that has `ChangeDetectorRef` as a dependency', () => {
// Note: we specifically check an @Injectable with `ChangeDetectorRef` here due to the fact that
// in Ivy there is a special instruction that injects `ChangeDetectorRef` token for Pipes
// (ɵɵinjectPipeChangeDetectorRef) and using that function for other types causes problems,
// for example when we try to override an @Injectable. The test below captures a use-case that
// triggers a problem in case incompatible function is used to inject `ChangeDetectorRef` as a
// dependency.
@Injectable({providedIn: 'root'})
class MyService {
token = 'original';