From e17bde99f875c1770e10b78d191e3039c7d3be38 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 15 Feb 2020 12:10:56 +0100 Subject: [PATCH] fix(ivy): error in AOT when pipe inherits constructor from injectable that uses DI (#35468) When a pipe inherits its constructor, and as a result its factory, from an injectable in AOT mode, it can end up throwing an error, because the inject implementation hasn't been set yet. These changes ensure that the implementation is set before the pipe's factory is invoked. Note that this isn't a problem in JIT mode, because the factory inheritance works slightly differently, hence why this test isn't going through `TestBed`. Fixes #35277. PR Close #35468 --- packages/core/src/render3/pipe.ts | 5 ++- packages/core/test/render3/pipe_spec.ts | 51 +++++++++++++++++++++++-- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/packages/core/src/render3/pipe.ts b/packages/core/src/render3/pipe.ts index a7b1452b91..d7e79f0dbd 100644 --- a/packages/core/src/render3/pipe.ts +++ b/packages/core/src/render3/pipe.ts @@ -8,9 +8,10 @@ import {WrappedValue} from '../change_detection/change_detection_util'; import {PipeTransform} from '../change_detection/pipe_transform'; +import {setInjectImplementation} from '../di/injector_compatibility'; import {getFactoryDef} from './definition'; -import {store} from './instructions/all'; +import {store, ɵɵdirectiveInject} from './instructions/all'; import {PipeDef, PipeDefList} from './interfaces/definition'; import {HEADER_OFFSET, LView, TVIEW} from './interfaces/view'; import {pureFunction1Internal, pureFunction2Internal, pureFunction3Internal, pureFunction4Internal, pureFunctionVInternal} from './pure_function'; @@ -45,7 +46,9 @@ export function ɵɵpipe(index: number, pipeName: string): any { } const pipeFactory = pipeDef.factory || (pipeDef.factory = getFactoryDef(pipeDef.type, true)); + const previousInjectImplementation = setInjectImplementation(ɵɵdirectiveInject); const pipeInstance = pipeFactory(); + setInjectImplementation(previousInjectImplementation); store(tView, getLView(), index, pipeInstance); return pipeInstance; } diff --git a/packages/core/test/render3/pipe_spec.ts b/packages/core/test/render3/pipe_spec.ts index ee8ff71724..06584ce27f 100644 --- a/packages/core/test/render3/pipe_spec.ts +++ b/packages/core/test/render3/pipe_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Directive as _Directive, Pipe as _Pipe, PipeTransform, WrappedValue, ɵɵdefinePipe} from '@angular/core'; +import {Injectable as _Injectable, Pipe as _Pipe, PipeTransform, WrappedValue, ɵɵdefineInjectable, ɵɵdefinePipe, ɵɵgetInheritedFactory, ɵɵinject} from '@angular/core'; import {expect} from '@angular/platform-browser/testing/src/matchers'; import {ɵɵtext, ɵɵtextInterpolate1} from '../../src/render3/instructions/all'; @@ -19,11 +19,16 @@ const Pipe: typeof _Pipe = function(...args: any[]): any { return () => undefined; } as any; +const Injectable: typeof _Injectable = function(...args: any[]): any { + // In test we use @Injectable for documentation only so it's safe to mock out the implementation. + return () => undefined; +} as any; + -// TODO: hasn't been moved over into acceptance, because the `WrappedValue` tests need to -// use an impure pipe which always throws "changed after checked errors" with `TestBed` -// both in Ivy and ViewEngine. describe('pipe', () => { + // TODO: hasn't been moved over into acceptance, because the `WrappedValue` tests need to + // use an impure pipe which always throws "changed after checked errors" with `TestBed` + // both in Ivy and ViewEngine. describe('WrappedValue', () => { @Pipe({name: 'wrappingPipe'}) class WrappingPipe implements PipeTransform { @@ -59,4 +64,42 @@ describe('pipe', () => { }); }); + // This test isn't in `acceptance`, because we can't capture the same behavior that we want + // when going through `TestBed`. Here we're testing the behavior of AOT-compiled code which + // differs from the JIT code in `TestBed`, because it includes a `ɵɵgetInheritedFactory` call + // when the pipe is using inheritance. + it('should be able to use DI in a Pipe that extends an Injectable', () => { + @Injectable({providedIn: 'root'}) + class SayHelloService { + getHello() { return 'Hello there'; } + static ɵfac = () => new SayHelloService(); + static ɵprov = ɵɵdefineInjectable( + {token: SayHelloService, factory: SayHelloService.ɵfac, providedIn: 'root'}); + } + + @Injectable() + class ParentPipe { + constructor(protected sayHelloService: SayHelloService) {} + static ɵfac = (t?: any) => new (t || ParentPipe)(ɵɵinject(SayHelloService)); + static ɵprov = ɵɵdefineInjectable({token: ParentPipe, factory: ParentPipe.ɵfac}); + } + + @Pipe({name: 'sayHello', pure: true}) + class SayHelloPipe extends ParentPipe implements PipeTransform { + transform() { return this.sayHelloService.getHello(); } + static ɵfac = (t?: any) => ɵɵgetInheritedFactory(t || SayHelloPipe)(SayHelloPipe); + static ɵpipe = ɵɵdefinePipe({name: 'sayHello', type: SayHelloPipe, pure: true}); + } + + const fixture = new TemplateFixture( + () => { + ɵɵtext(0); + ɵɵpipe(1, 'sayHello'); + }, + () => { ɵɵtextInterpolate1('', ɵɵpipeBind1(1, 1, null), ''); }, 2, 3, undefined, + [SayHelloPipe]); + + expect(fixture.html).toBe('Hello there'); + }); + });