From efc67ee5ef5ec5d4fb18f0995e21fb12ecee52ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mi=C5=A1ko=20Hevery?= Date: Mon, 5 Feb 2018 13:28:38 -0800 Subject: [PATCH] fix(ivy): make pipe invocation locality neutral (#22030) PR Close #22030 --- packages/core/src/metadata/directives.ts | 10 ++++---- packages/core/src/render3/pipe.ts | 10 ++++---- .../test/render3/compiler_canonical_spec.ts | 25 ++++++++++--------- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/packages/core/src/metadata/directives.ts b/packages/core/src/metadata/directives.ts index 43f340c60a..25571fcb54 100644 --- a/packages/core/src/metadata/directives.ts +++ b/packages/core/src/metadata/directives.ts @@ -795,11 +795,11 @@ export interface Pipe { /** * If Pipe is pure (its output depends only on its input.) * - * Normally pipe's `translate` method is invoked on each change detection - * cycle. If the cost of invoking the `translated` method is non-trivial and - * if the output of the pipe depends only on its input, then declaring a pipe - * as pure would short circuit the invocation of the `translate` method and - * invoke the method only when the inputs to the pipe change. + * Normally pipe's `transform` method is only invoked when the inputs to pipe`s + * `transform` method change. If the pipe has internal state (it's result are + * dependant on state other than its arguments) than set `pure` to `false` so + * that the pipe is invoked on each change-detection even if the arguments to the + * pipe do not change. */ pure?: boolean; } diff --git a/packages/core/src/render3/pipe.ts b/packages/core/src/render3/pipe.ts index a915a17c73..6af5df0630 100644 --- a/packages/core/src/render3/pipe.ts +++ b/packages/core/src/render3/pipe.ts @@ -20,7 +20,7 @@ export function pipe(index: number, pipeDef: PipeDef, pipe: T): void { } /** - * Invokes a pure pipe with 4 arguments. + * Invokes a pipe with 1 arguments. * * This instruction acts as a guard to {@link PipeTransform#transform} invoking * the pipe only when an input to the pipe changes. @@ -33,7 +33,7 @@ export function pipeBind1(index: number, v1: any): any { } /** - * Invokes a pure pipe with 4 arguments. + * Invokes a pipe with 2 arguments. * * This instruction acts as a guard to {@link PipeTransform#transform} invoking * the pipe only when an input to the pipe changes. @@ -47,7 +47,7 @@ export function pipeBind2(index: number, v1: any, v2: any): any { } /** - * Invokes a pure pipe with 4 arguments. + * Invokes a pipe with 3 arguments. * * This instruction acts as a guard to {@link PipeTransform#transform} invoking * the pipe only when an input to the pipe changes. @@ -62,7 +62,7 @@ export function pipeBind3(index: number, v1: any, v2: any, v3: any): any { } /** - * Invokes a pure pipe with 4 arguments. + * Invokes a pipe with 4 arguments. * * This instruction acts as a guard to {@link PipeTransform#transform} invoking * the pipe only when an input to the pipe changes. @@ -78,7 +78,7 @@ export function pipeBind4(index: number, v1: any, v2: any, v3: any, v4: any): an } /** - * Invokes a pure pipe with variable number of arguments. + * Invokes a pipe with variable number of arguments. * * This instruction acts as a guard to {@link PipeTransform#transform} invoking * the pipe only when an input to the pipe changes. diff --git a/packages/core/test/render3/compiler_canonical_spec.ts b/packages/core/test/render3/compiler_canonical_spec.ts index 86a6f0e01f..c1263e08d8 100644 --- a/packages/core/test/render3/compiler_canonical_spec.ts +++ b/packages/core/test/render3/compiler_canonical_spec.ts @@ -442,6 +442,7 @@ describe('compiler specification', () => { xdescribe('pipes', () => { @Pipe({ name: 'myPipe', + pure: false, }) class MyPipe implements PipeTransform, OnDestroy { @@ -449,14 +450,16 @@ describe('compiler specification', () => { ngOnDestroy(): void { throw new Error('Method not implemented.'); } // NORMATIVE - static ngPipeDef = r3.definePipe( - {type: MyPipe, factory: function MyPipe_Factory() { return new MyPipe(); }}); + static ngPipeDef = r3.definePipe({ + type: MyPipe, + factory: function MyPipe_Factory() { return new MyPipe(); }, + pure: false, + }); // /NORMATIVE } @Pipe({ name: 'myPurePipe', - pure: true, }) class MyPurePipe implements PipeTransform { transform(value: any, ...args: any[]) { throw new Error('Method not implemented.'); } @@ -465,7 +468,6 @@ describe('compiler specification', () => { static ngPipeDef = r3.definePipe({ type: MyPurePipe, factory: function MyPurePipe_Factory() { return new MyPurePipe(); }, - pure: true }); // /NORMATIVE } @@ -482,19 +484,18 @@ describe('compiler specification', () => { factory: function MyApp_Factory() { return new MyApp(); }, template: function MyApp_Template(ctx: MyApp, cm: boolean) { if (cm) { - r3.Pp(0, MyPipe_ngPipeDef, MyPipe_ngPipeDef.n()); + r3.T(0); r3.Pp(1, MyPurePipe_ngPipeDef, MyPurePipe_ngPipeDef.n()); - r3.T(2); + r3.Pp(2, MyPipe_ngPipeDef, MyPipe_ngPipeDef.n()); } - r3.t( - 2, r3.b1('', r3.pb2(1, r3.m(0).transform(ctx.name, ctx.size), ctx.size), '')); + r3.t(2, r3.b1('', r3.pb2(1, r3.pb2(2, ctx.name, ctx.size), ctx.size), '')); } }); // /NORMATIVE } // NORMATIVE - const MyPipe_ngPipeDef = MyPipe.ngPipeDef; const MyPurePipe_ngPipeDef = MyPurePipe.ngPipeDef; + const MyPipe_ngPipeDef = MyPipe.ngPipeDef; // /NORMATIVE it('should render pipes', () => { @@ -827,7 +828,7 @@ xdescribe('NgModule', () => { constructor(name: String) {} // NORMATIVE static ngInjectableDef = defineInjectable({ - factory: () => new Toast(inject(String)), + factory: () => new Toast(r3.inject(String)), }); // /NORMATIVE } @@ -846,7 +847,7 @@ xdescribe('NgModule', () => { constructor(toast: Toast) {} // NORMATIVE static ngInjectorDef = defineInjector({ - factory: () => new MyModule(inject(Toast)), + factory: () => new MyModule(r3.inject(Toast)), provider: [ {provide: Toast, deps: [String]}, // If Toast has metadata generate this line Toast, // If Toast has no metadata generate this line. @@ -863,7 +864,7 @@ xdescribe('NgModule', () => { // NORMATIVE static ngInjectableDef = defineInjectable({ scope: MyModule, - factory: () => new BurntToast(inject(Toast, r3.InjectFlags.Optional), inject(String)), + factory: () => new BurntToast(r3.inject(Toast, r3.InjectFlags.Optional), r3.inject(String)), }); // /NORMATIVE }