iSharkFly-Docs
/
angular-docs-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(ivy): incorrect ChangeDetectorRef injected into pipes used in component inputs (#31438) 

When injecting a `ChangeDetectorRef` into a pipe, the expected result is that the ref will be tied to the component in which the pipe is being used. This works for most cases, however when a pipe is used inside a property binding of a component (see test case as an example), the current `TNode` is pointing to component's host so we end up injecting the inner component's view. These changes fix the issue by only looking up the component view of the `TNode` if the `TNode` is a parent.

This PR resolves FW-1419.

PR Close #31438
This commit is contained in:
crisbeto 2019-07-12 20:15:12 +02:00 committed by Kara Erickson
parent f50dede8f7
commit 0aff4a6919
16 changed files with 246 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
packages/compiler-cli/src/ngtsc/annotations/src/util.ts
View File

@ -49,6 +49,7 @@ export function getConstructorDependencies(
let token = valueReferenceToExpression(param.typeValueReference, defaultImportRecorder);
let optional = false, self = false, skipSelf = false, host = false;
let resolved = R3ResolvedDependencyType.Token;
(param.decorators || []).filter(dec => isCore || isAngularCore(dec)).forEach(dec => {
const name = isCore || dec.import === null ? dec.name : dec.import !.name;
if (name === 'Inject') {
@ -79,6 +80,11 @@ export function getConstructorDependencies(
ErrorCode.DECORATOR_UNEXPECTED, dec.node, `Unexpected decorator ${name} on parameter.`);
}
});
if (token instanceof ExternalExpr && token.value.name === 'ChangeDetectorRef' &&
token.value.moduleName === '@angular/core') {
resolved = R3ResolvedDependencyType.ChangeDetectorRef;
}
if (token === null) {
errors.push({
index: idx,

61
packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts
View File

@ -2145,6 +2145,67 @@ describe('compiler compliance', () => {
expectEmit(source, MyAppDefinition, 'Invalid MyApp definition');
});
it('should generate the proper instruction when injecting ChangeDetectorRef into a pipe',
() => {
const files = {
app: {
'spec.ts': `
import {Component, NgModule, Pipe, PipeTransform, ChangeDetectorRef, Optional} from '@angular/core';
@Pipe({name: 'myPipe'})
export class MyPipe implements PipeTransform {
constructor(changeDetectorRef: ChangeDetectorRef) {}
transform(value: any, ...args: any[]) { return value; }
}
@Pipe({name: 'myOtherPipe'})
export class MyOtherPipe implements PipeTransform {
constructor(@Optional() changeDetectorRef: ChangeDetectorRef) {}
transform(value: any, ...args: any[]) { return value; }
}
@Component({
selector: 'my-app',
template: '{{name | myPipe }}<p>{{ name | myOtherPipe }}</p>'
})
export class MyApp {
name = 'World';
}
@NgModule({declarations:[MyPipe, MyOtherPipe, MyApp]})
export class MyModule {}
`
}
};
const MyPipeDefinition = `
MyPipe.ngPipeDef = $r3$.ɵɵdefinePipe({
name: "myPipe",
type: MyPipe,
factory: function MyPipe_Factory(t) { return new (t || MyPipe)($r3$.ɵɵinjectPipeChangeDetectorRef()); },
pure: true
});
`;
const MyOtherPipeDefinition = `
MyOtherPipe.ngPipeDef = $r3$.ɵɵdefinePipe({
name: "myOtherPipe",
type: MyOtherPipe,
factory: function MyOtherPipe_Factory(t) { return new (t || MyOtherPipe)($r3$.ɵɵinjectPipeChangeDetectorRef(8)); },
pure: true
});`;
const result = compile(files, angularFiles);
const source = result.source;
expectEmit(source, MyPipeDefinition, 'Invalid pipe definition');
expectEmit(source, MyOtherPipeDefinition, 'Invalid alternate pipe definition');
});
});
it('local reference', () => {

1
packages/compiler/src/compiler_facade_interface.ts
View File

@ -65,6 +65,7 @@ export type Provider = any;
export enum R3ResolvedDependencyType {
Token = 0,
Attribute = 1,
ChangeDetectorRef = 2,
}
export interface R3DependencyMetadataFacade {

40
packages/compiler/src/render3/r3_factory.ts
View File

@ -97,6 +97,11 @@ export enum R3ResolvedDependencyType {
* The token expression is a string representing the attribute name.
*/
Attribute = 1,
/**
* Injecting the `ChangeDetectorRef` token. Needs special handling when injected into a pipe.
*/
ChangeDetectorRef = 2,
}
/**
@ -138,8 +143,8 @@ export interface R3DependencyMetadata {
/**
* Construct a factory function expression for the given `R3FactoryMetadata`.
*/
export function compileFactoryFunction(meta: R3FactoryMetadata):
{factory: o.Expression, statements: o.Statement[]} {
export function compileFactoryFunction(
meta: R3FactoryMetadata, isPipe = false): {factory: o.Expression, statements: o.Statement[]} {
const t = o.variable('t');
const statements: o.Statement[] = [];
@ -155,7 +160,8 @@ export function compileFactoryFunction(meta: R3FactoryMetadata):
if (meta.deps !== null) {
// There is a constructor (either explicitly or implicitly defined).
if (meta.deps !== 'invalid') {
ctorExpr = new o.InstantiateExpr(typeForCtor, injectDependencies(meta.deps, meta.injectFn));
ctorExpr =
new o.InstantiateExpr(typeForCtor, injectDependencies(meta.deps, meta.injectFn, isPipe));
}
} else {
const baseFactory = o.variable(`ɵ${meta.name}_BaseFactory`);
@ -203,7 +209,7 @@ export function compileFactoryFunction(meta: R3FactoryMetadata):
} else if (isDelegatedMetadata(meta)) {
// This type is created with a delegated factory. If a type parameter is not specified, call
// the factory instead.
const delegateArgs = injectDependencies(meta.delegateDeps, meta.injectFn);
const delegateArgs = injectDependencies(meta.delegateDeps, meta.injectFn, isPipe);
// Either call `new delegate(...)` or `delegate(...)` depending on meta.useNewForDelegate.
const factoryExpr = new (
meta.delegateType === R3FactoryDelegateType.Class ?
@ -232,30 +238,38 @@ export function compileFactoryFunction(meta: R3FactoryMetadata):
}
function injectDependencies(
deps: R3DependencyMetadata[], injectFn: o.ExternalReference): o.Expression[] {
return deps.map(dep => compileInjectDependency(dep, injectFn));
deps: R3DependencyMetadata[], injectFn: o.ExternalReference, isPipe: boolean): o.Expression[] {
return deps.map(dep => compileInjectDependency(dep, injectFn, isPipe));
}
function compileInjectDependency(
dep: R3DependencyMetadata, injectFn: o.ExternalReference): o.Expression {
dep: R3DependencyMetadata, injectFn: o.ExternalReference, isPipe: boolean): o.Expression {
// Interpret the dependency according to its resolved type.
switch (dep.resolved) {
case R3ResolvedDependencyType.Token: {
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);
// Build up the arguments to the injectFn call.
const injectArgs = [dep.token];
// 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
// that's being used.
if (flags !== InjectFlags.Default || dep.optional) {
injectArgs.push(o.literal(flags));
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) {
injectArgs.push(flagsParam);
}
return o.importExpr(injectFn).callFn(injectArgs);
}
case R3ResolvedDependencyType.Attribute:
// In the case of attributes, the attribute name in question is given as the token.
return o.importExpr(R3.injectAttribute).callFn([dep.token]);

3
packages/compiler/src/render3/r3_identifiers.ts
View File

@ -215,6 +215,9 @@ 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 templateRefExtractor:

14
packages/compiler/src/render3/r3_pipe_compiler.ts
View File

@ -57,12 +57,14 @@ export function compilePipeFromMetadata(metadata: R3PipeMetadata) {
// e.g. `type: MyPipe`
definitionMapValues.push({key: 'type', value: metadata.type, quoted: false});
const templateFactory = compileFactoryFunction({
name: metadata.name,
type: metadata.type,
deps: metadata.deps,
injectFn: R3.directiveInject,
});
const templateFactory = compileFactoryFunction(
{
name: metadata.name,
type: metadata.type,
deps: metadata.deps,
injectFn: R3.directiveInject,
},
true);
definitionMapValues.push({key: 'factory', value: templateFactory.factory, quoted: false});
// e.g. `pure: true`

1
packages/core/src/compiler/compiler_facade_interface.ts
View File

@ -65,6 +65,7 @@ export type Provider = any;
export enum R3ResolvedDependencyType {
Token = 0,
Attribute = 1,
ChangeDetectorRef = 2,
}
export interface R3DependencyMetadataFacade {

1
packages/core/src/core_render3_private_export.ts
View File

@ -33,6 +33,7 @@ export {
RenderFlags as ɵRenderFlags,
ɵɵdirectiveInject,
ɵɵinjectAttribute,
ɵɵinjectPipeChangeDetectorRef,
ɵɵgetFactoryOf,
ɵɵgetInheritedFactory,
ɵɵsetComponentScope,

4
packages/core/src/di/jit/util.ts
View File

@ -6,6 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {ChangeDetectorRef} from '../../change_detection/change_detector_ref';
import {CompilerFacade, R3DependencyMetadataFacade, getCompilerFacade} from '../../compiler/compiler_facade';
import {Type} from '../../interface/type';
import {ReflectionCapabilities} from '../../reflection/reflection_capabilities';
@ -66,6 +67,9 @@ function reflectDependency(compiler: CompilerFacade, dep: any | any[]): R3Depend
}
meta.token = param.attributeName;
meta.resolved = compiler.R3ResolvedDependencyType.Attribute;
} else if (param === ChangeDetectorRef) {
meta.token = param;
meta.resolved = compiler.R3ResolvedDependencyType.ChangeDetectorRef;
} else {
setTokenAndResolvedType(param);
}

3
packages/core/src/render3/di.ts
View File

@ -6,10 +6,11 @@
* found in the LICENSE file at https://angular.io/license
*/
import {InjectFlags, InjectionToken} from '../di';
import {InjectionToken} from '../di/injection_token';
import {Injector} from '../di/injector';
import {injectRootLimpMode, setInjectImplementation} from '../di/injector_compatibility';
import {getInjectableDef, getInjectorDef} from '../di/interface/defs';
import {InjectFlags} from '../di/interface/injector';
import {Type} from '../interface/type';
import {assertDefined, assertEqual} from '../util/assert';

2
packages/core/src/render3/index.ts
View File

@ -198,7 +198,7 @@ export {
ɵɵpureFunctionV,
} from './pure_function';
export {ɵɵtemplateRefExtractor} from './view_engine_compatibility_prebound';
export {ɵɵtemplateRefExtractor, ɵɵinjectPipeChangeDetectorRef} from './view_engine_compatibility_prebound';
export {ɵɵresolveWindow, ɵɵresolveDocument, ɵɵresolveBody} from './util/misc_utils';

1
packages/core/src/render3/jit/environment.ts
View File

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

14
packages/core/src/render3/view_engine_compatibility.ts
View File

@ -362,8 +362,8 @@ export function createContainerRef(
/** Returns a ChangeDetectorRef (a.k.a. a ViewRef) */
export function injectChangeDetectorRef(): ViewEngine_ChangeDetectorRef {
return createViewRef(getPreviousOrParentTNode(), getLView(), null);
export function injectChangeDetectorRef(isPipe = false): ViewEngine_ChangeDetectorRef {
return createViewRef(getPreviousOrParentTNode(), getLView(), isPipe);
}
/**
@ -371,15 +371,15 @@ export function injectChangeDetectorRef(): ViewEngine_ChangeDetectorRef {
*
* @param hostTNode The node that is requesting a ChangeDetectorRef
* @param hostView The view to which the node belongs
* @param context The context for this change detector ref
* @param isPipe Whether the view is being injected into a pipe.
* @returns The ChangeDetectorRef to use
*/
export function createViewRef(
hostTNode: TNode, hostView: LView, context: any): ViewEngine_ChangeDetectorRef {
if (isComponent(hostTNode)) {
function createViewRef(
hostTNode: TNode, hostView: LView, isPipe: boolean): ViewEngine_ChangeDetectorRef {
if (isComponent(hostTNode) && !isPipe) {
const componentIndex = hostTNode.directiveStart;
const componentView = getComponentViewByIndex(hostTNode.index, hostView);
return new ViewRef(componentView, context, componentIndex);
return new ViewRef(componentView, null, componentIndex);
} else if (
hostTNode.type === TNodeType.Element || hostTNode.type === TNodeType.Container ||
hostTNode.type === TNodeType.ElementContainer) {

19
packages/core/src/render3/view_engine_compatibility_prebound.ts
View File

@ -7,12 +7,14 @@
*/
import {ChangeDetectorRef} from '../change_detection/change_detector_ref';
import {InjectFlags} from '../di/interface/injector';
import {ElementRef as ViewEngine_ElementRef} from '../linker/element_ref';
import {TemplateRef as ViewEngine_TemplateRef} from '../linker/template_ref';
import {TNode} from './interfaces/node';
import {LView} from './interfaces/view';
import {createTemplateRef} from './view_engine_compatibility';
import {createTemplateRef, injectChangeDetectorRef} from './view_engine_compatibility';
@ -25,3 +27,18 @@ import {createTemplateRef} from './view_engine_compatibility';
export function ɵɵtemplateRefExtractor(tNode: TNode, currentView: LView) {
return createTemplateRef(ViewEngine_TemplateRef, ViewEngine_ElementRef, tNode, currentView);
}
/**
* 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)) {
throw new Error(`No provider for ChangeDetectorRef!`);
} else {
return value;
}
}

104
packages/core/test/acceptance/pipe_spec.ts
View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {Component, Directive, Inject, Injectable, InjectionToken, Input, NgModule, OnDestroy, Pipe, PipeTransform, ViewChild} from '@angular/core';
import {ChangeDetectionStrategy, ChangeDetectorRef, Component, Directive, Inject, Injectable, InjectionToken, Input, NgModule, OnDestroy, Pipe, PipeTransform, ViewChild} from '@angular/core';
import {TestBed} from '@angular/core/testing';
import {expect} from '@angular/platform-browser/testing/src/matchers';
@ -398,6 +398,108 @@ describe('pipe', () => {
expect(fixture.nativeElement.textContent).toBe('MyComponent Title - Service Title');
});
it('should inject the ChangeDetectorRef of the containing view when using pipe inside a component input',
() => {
let pipeChangeDetectorRef: ChangeDetectorRef|undefined;
@Component({
changeDetection: ChangeDetectionStrategy.OnPush,
selector: 'some-comp',
template: 'Inner value: "{{displayValue}}"',
})
class SomeComp {
@Input() value: any;
displayValue = 0;
}
@Component({
changeDetection: ChangeDetectionStrategy.OnPush,
template: `
<some-comp [value]="pipeValue | testPipe"></some-comp>
Outer value: "{{displayValue}}"
`,
})
class App {
@Input() something: any;
@ViewChild(SomeComp, {static: false}) comp !: SomeComp;
pipeValue = 10;
displayValue = 0;
}
@Pipe({name: 'testPipe'})
class TestPipe implements PipeTransform {
constructor(changeDetectorRef: ChangeDetectorRef) {
pipeChangeDetectorRef = changeDetectorRef;
}
transform() { return ''; }
}
TestBed.configureTestingModule({declarations: [App, SomeComp, TestPipe]});
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
fixture.componentInstance.displayValue = 1;
fixture.componentInstance.comp.displayValue = 1;
pipeChangeDetectorRef !.markForCheck();
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toContain('Outer value: "1"');
expect(fixture.nativeElement.textContent).toContain('Inner value: "0"');
});
it('should inject the ChangeDetectorRef of the containing view when using pipe inside a component input which has child nodes',
() => {
let pipeChangeDetectorRef: ChangeDetectorRef|undefined;
@Component({
changeDetection: ChangeDetectionStrategy.OnPush,
selector: 'some-comp',
template: 'Inner value: "{{displayValue}}" <ng-content></ng-content>',
})
class SomeComp {
@Input() value: any;
displayValue = 0;
}
@Component({
changeDetection: ChangeDetectionStrategy.OnPush,
template: `
<some-comp [value]="pipeValue | testPipe">
<div>Hello</div>
</some-comp>
Outer value: "{{displayValue}}"
`,
})
class App {
@Input() something: any;
@ViewChild(SomeComp, {static: false}) comp !: SomeComp;
pipeValue = 10;
displayValue = 0;
}
@Pipe({name: 'testPipe'})
class TestPipe implements PipeTransform {
constructor(changeDetectorRef: ChangeDetectorRef) {
pipeChangeDetectorRef = changeDetectorRef;
}
transform() { return ''; }
}
TestBed.configureTestingModule({declarations: [App, SomeComp, TestPipe]});
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
fixture.componentInstance.displayValue = 1;
fixture.componentInstance.comp.displayValue = 1;
pipeChangeDetectorRef !.markForCheck();
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toContain('Outer value: "1"');
expect(fixture.nativeElement.textContent).toContain('Inner value: "0"');
});
});
});

2
tools/public_api_guard/core/core.d.ts vendored
View File

@ -920,6 +920,8 @@ export interface ɵɵInjectorDef<T> {
providers: (Type<any> | ValueProvider | ExistingProvider | FactoryProvider | ConstructorProvider | StaticClassProvider | ClassProvider | any[])[];
}
export declare function ɵɵinjectPipeChangeDetectorRef(flags?: InjectFlags): ChangeDetectorRef | null;
export declare function ɵɵlistener(eventName: string, listenerFn: (e?: any) => any, useCapture?: boolean, eventTargetResolver?: GlobalTargetResolver): void;
export declare function ɵɵload<T>(index: number): T;