refactor(ivy): generate pipe names instead of defs (#23104)

PR Close #23104

packages/compiler/src/render3/r3_pipe_compiler.ts

@@ -24,7 +24,10 @@ export function compilePipe(
     mode: OutputMode) {
   const definitionMapValues: {key: string, quoted: boolean, value: o.Expression}[] = [];
 
-  // e.g. 'type: MyPipe`
+  // e.g. `name: 'myPipe'`
+  definitionMapValues.push({key: 'name', value: o.literal(pipe.name), quoted: false});
+
+  // e.g. `type: MyPipe`
   definitionMapValues.push(
       {key: 'type', value: outputCtx.importExpr(pipe.type.reference), quoted: false});
 

packages/compiler/src/render3/r3_view_compiler.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {CompileDirectiveMetadata, CompileDirectiveSummary, CompilePipeSummary, CompileQueryMetadata, CompileTokenMetadata, CompileTypeMetadata, flatten, identifierName, rendererTypeName, sanitizeIdentifier, tokenReference, viewClassName} from '../compile_metadata';
+import {CompileDirectiveMetadata, CompileDirectiveSummary, CompilePipeSummary, CompileQueryMetadata, CompileTokenMetadata, CompileTypeMetadata, CompileTypeSummary, flatten, identifierName, rendererTypeName, sanitizeIdentifier, tokenReference, viewClassName} from '../compile_metadata';
 import {CompileReflector} from '../compile_reflector';
 import {BindingForm, BuiltinConverter, BuiltinFunctionCall, ConvertPropertyBindingResult, EventHandlerVars, LocalResolver, convertActionBinding, convertPropertyBinding, convertPropertyBindingBuiltins} from '../compiler_util/expression_converter';
 import {ConstantPool, DefinitionKind} from '../constant_pool';
@@ -111,6 +111,14 @@ export function compileComponent(
     template: TemplateAst[], reflector: CompileReflector, bindingParser: BindingParser,
     mode: OutputMode) {
   const definitionMapValues: {key: string, quoted: boolean, value: o.Expression}[] = [];
+  // Set of pipe names for pipe exps that have already been stored in pipes[] (to avoid dupes)
+  const pipeSet = new Set<string>();
+  // Pipe expressions for pipes[] field in component def
+  const pipeExps: o.Expression[] = [];
+
+  function addPipeDependency(summary: CompilePipeSummary): void {
+    addDependencyToComponent(outputCtx, summary, pipeSet, pipeExps);
+  }
 
   const field = (key: string, value: o.Expression | null) => {
     if (value) {
@@ -154,11 +162,16 @@ export function compileComponent(
       new TemplateDefinitionBuilder(
           outputCtx, outputCtx.constantPool, reflector, CONTEXT_NAME, ROOT_SCOPE.nestedScope(), 0,
           component.template !.ngContentSelectors, templateTypeName, templateName, pipeMap,
-          component.viewQueries)
+          component.viewQueries, addPipeDependency)
           .buildTemplateFunction(template, []);
 
   field('template', templateFunctionExpression);
 
+  // e.g. `pipes: [MyPipe]`
+  if (pipeExps.length) {
+    field('pipes', o.literalArr(pipeExps));
+  }
+
   // e.g `inputs: {a: 'a'}`
   field('inputs', createInputsObject(component, outputCtx));
 
@@ -201,6 +214,19 @@ export function compileComponent(
   }
 }
 
+// TODO: this should be used for addDirectiveDependency as well when Misko's PR goes in
+function addDependencyToComponent(
+    outputCtx: OutputContext, summary: CompileTypeSummary, set: Set<string>,
+    exps: o.Expression[]): void {
+  const importExpr = outputCtx.importExpr(summary.type.reference) as o.ExternalExpr;
+  const uniqueKey = importExpr.value.moduleName + ':' + importExpr.value.name;
+
+  if (!set.has(uniqueKey)) {
+    set.add(uniqueKey);
+    exps.push(importExpr);
+  }
+}
+
 // TODO: Remove these when the things are fully supported
 function unknown<T>(arg: o.Expression | o.Statement | TemplateAst): never {
   throw new Error(
@@ -347,20 +373,16 @@ class TemplateDefinitionBuilder implements TemplateAstVisitor, LocalResolver {
       private reflector: CompileReflector, private contextParameter: string,
       private bindingScope: BindingScope, private level = 0, private ngContentSelectors: string[],
       private contextName: string|null, private templateName: string|null,
-      private pipes: Map<string, CompilePipeSummary>, private viewQueries: CompileQueryMetadata[]) {
+      private pipes: Map<string, CompilePipeSummary>, private viewQueries: CompileQueryMetadata[],
+      private addPipeDependency: (summary: CompilePipeSummary) => void) {
     this._valueConverter = new ValueConverter(
         outputCtx, () => this.allocateDataSlot(), (name, localName, slot, value) => {
           bindingScope.set(localName, value);
           const pipe = pipes.get(name) !;
           pipe || error(`Could not find pipe ${name}`);
-          const pipeDefinition = constantPool.getDefinition(
+          this.addPipeDependency(pipe);
-              pipe.type.reference, DefinitionKind.Pipe, outputCtx, /* forceShared */ true);
           this._creationMode.push(
-              o.importExpr(R3.pipe)
+              o.importExpr(R3.pipe).callFn([o.literal(slot), o.literal(name)]).toStmt());
-                  .callFn([
-                    o.literal(slot), pipeDefinition, pipeDefinition.callMethod(R3.NEW_METHOD, [])
-                  ])
-                  .toStmt());
         });
   }
 
@@ -736,7 +758,7 @@ class TemplateDefinitionBuilder implements TemplateAstVisitor, LocalResolver {
     const templateVisitor = new TemplateDefinitionBuilder(
         this.outputCtx, this.constantPool, this.reflector, templateContext,
         this.bindingScope.nestedScope(), this.level + 1, this.ngContentSelectors, contextName,
-        templateName, this.pipes, []);
+        templateName, this.pipes, [], this.addPipeDependency);
     const templateFunctionExpr = templateVisitor.buildTemplateFunction(ast.children, ast.variables);
     this._postfix.push(templateFunctionExpr.toDeclStmt(templateName, null));
   }
@@ -1044,11 +1066,7 @@ class ValueConverter extends AstMemoryEfficientTransformer {
   // AstMemoryEfficientTransformer
   visitPipe(ast: BindingPipe, context: any): AST {
     // Allocate a slot to create the pipe
-    let slot = this.pipeSlots.get(ast.name);
+    const slot = this.allocateSlot();
-    if (slot == null) {
-      slot = this.allocateSlot();
-      this.pipeSlots.set(ast.name, slot);
-    }
     const slotPseudoLocal = `PIPE:${slot}`;
     const target = new PropertyRead(ast.span, new ImplicitReceiver(ast.span), slotPseudoLocal);
     const bindingId = pipeBinding(ast.args);

packages/compiler/test/render3/r3_compiler_compliance_spec.ts

@@ -97,7 +97,6 @@ describe('compiler compliance', () => {
           template: function ChildComponent_Template(ctx: IDENT, cm: IDENT) {
             if (cm) {
               $r3$.ɵT(0, 'child-view');
-           
             }
           }
         });`;
@@ -784,7 +783,10 @@ describe('compiler compliance', () => {
                 transform(value: any, ...args: any[]) { return value; }
               }
 
-              @Component({selector: 'my-app', template: '{{name | myPipe:size | myPurePipe:size }}'})
+              @Component({
+                selector: 'my-app', 
+                template: '{{name | myPipe:size | myPurePipe:size }}<p>{{ name | myPurePipe:size }}</p>'
+              })
               export class MyApp {
                 name = 'World';
                 size = 0;
@@ -799,19 +801,18 @@ describe('compiler compliance', () => {
       it('should render pipes', () => {
         const MyPipeDefinition = `
             static ngPipeDef = $r3$.ɵdefinePipe(
-                {type: MyPipe, factory: function MyPipe_Factory() { return new MyPipe(); }});
+                {name: 'myPipe', type: MyPipe, factory: function MyPipe_Factory() { return new MyPipe(); }});
         `;
 
         const MyPurePipeDefinition = `
             static ngPipeDef = $r3$.ɵdefinePipe({
+              name: 'myPurePipe', 
               type: MyPurePipe,
               factory: function MyPurePipe_Factory() { return new MyPurePipe(); },
               pure: true
             });`;
 
         const MyAppDefinition = `
-            const $MyPurePipe_ngPipeDef$ = MyPurePipe.ngPipeDef;
-            const $MyPipe_ngPipeDef$ = MyPipe.ngPipeDef;
             …
             static ngComponentDef = $r3$.ɵdefineComponent({
               type: MyApp,
@@ -820,11 +821,17 @@ describe('compiler compliance', () => {
               template: function MyApp_Template(ctx: IDENT, cm: IDENT) {
                 if (cm) {
                   $r3$.ɵT(0);
-                  $r3$.ɵPp(1, $MyPurePipe_ngPipeDef$, $MyPurePipe_ngPipeDef$.n());
+                  $r3$.ɵPp(1, 'myPurePipe');
-                  $r3$.ɵPp(2, $MyPipe_ngPipeDef$, $MyPipe_ngPipeDef$.n());
+                  $r3$.ɵPp(2, 'myPipe');
+                  $r3$.ɵE(3, 'p');
+                  $r3$.ɵT(4);
+                  $r3$.ɵPp(5, 'myPurePipe');
+                  $r3$.ɵe();
                 }
                 $r3$.ɵt(0, $r3$.ɵi1('', $r3$.ɵpb2(1, $r3$.ɵpb2(2,ctx.name, ctx.size), ctx.size), ''));
-              }
+                $r3$.ɵt(4, $r3$.ɵi1('', $r3$.ɵpb2(5, ctx.name, ctx.size), ''));
+              },
+              pipes: [MyPurePipe, MyPipe]
             });`;
 
         const result = compile(files, angularFiles);

packages/core/src/render3/pipe.ts

@@ -16,11 +16,10 @@ import {pureFunction1, pureFunction2, pureFunction3, pureFunction4, pureFunction
  * Create a pipe.
  *
  * @param index Pipe index where the pipe will be stored.
- * @param pipeDef Pipe definition object for registering life cycle hooks.
+ * @param pipeName The name of the pipe
- * @param firstInstance (optional) The first instance of the pipe that can be reused for pure pipes.
  * @returns T the instance of the pipe.
  */
-export function pipe(index: number, pipeName: string, firstInstance?: any): any {
+export function pipe(index: number, pipeName: string): any {
   const tView = getTView();
   let pipeDef: PipeDef<any>;
 
@@ -34,7 +33,7 @@ export function pipe(index: number, pipeName: string, firstInstance?: any): any
     pipeDef = tView.data[index] as PipeDef<any>;
   }
 
-  const pipeInstance = pipeDef.pure && firstInstance ? firstInstance : pipeDef.n();
+  const pipeInstance = pipeDef.n();
   store(index, pipeInstance);
   return pipeInstance;
 }

packages/core/test/render3/compiler_canonical/pipes_spec.ts

@@ -155,12 +155,11 @@ describe('pipes', () => {
         selectors: [['my-app']],
         factory: function MyApp_Factory() { return new MyApp(); },
         template: function MyApp_Template(ctx: $MyApp$, cm: $boolean$) {
-          let $pi$: $any$;
           if (cm) {
             $r3$.ɵT(0);
-            $pi$ = $r3$.ɵPp(1, 'myPurePipe');
+            $r3$.ɵPp(1, 'myPurePipe');
             $r3$.ɵT(2);
-            $r3$.ɵPp(3, 'myPurePipe', $pi$);
+            $r3$.ɵPp(3, 'myPurePipe');
             $r3$.ɵC(4, C4, '', ['oneTimeIf', '']);
           }
           $r3$.ɵt(0, $r3$.ɵi1('', $r3$.ɵpb2(1, ctx.name, ctx.size), ''));
@@ -173,7 +172,7 @@ describe('pipes', () => {
             if (cm) {
               $r3$.ɵE(0, 'div');
               $r3$.ɵT(1);
-              $r3$.ɵPp(2, 'myPurePipe', $pi$);
+              $r3$.ɵPp(2, 'myPurePipe');
               $r3$.ɵe();
             }
             $r3$.ɵt(1, $r3$.ɵi1('', $r3$.ɵpb2(2, ctx.name, ctx.size), ''));

packages/core/test/render3/pipe_spec.ts

@@ -183,49 +183,6 @@ describe('pipe', () => {
       expect(renderToHtml(Template, person, null, defs)).toEqual('bart state:2');
       expect(renderToHtml(Template, person, null, defs)).toEqual('bart state:2');
     });
-
-    it('should cache pure pipes', () => {
-      function Template(ctx: any, cm: boolean) {
-        let pipeInstance: any;
-        if (cm) {
-          elementStart(0, 'div');
-          pipeInstance = pipe(1, 'countingPipe');
-          elementEnd();
-          elementStart(2, 'div');
-          pipe(3, 'countingPipe', pipeInstance);
-          elementEnd();
-          container(4);
-        }
-        elementProperty(0, 'someProp', bind(pipeBind1(1, true)));
-        elementProperty(2, 'someProp', bind(pipeBind1(3, true)));
-        pipeInstances.push(load<CountingPipe>(1), load(3));
-        containerRefreshStart(4);
-        {
-          for (let i of [1, 2]) {
-            let cm1 = embeddedViewStart(1);
-            {
-              if (cm1) {
-                elementStart(0, 'div');
-                pipe(1, 'countingPipe', pipeInstance);
-                elementEnd();
-              }
-              elementProperty(0, 'someProp', bind(pipeBind1(1, true)));
-              pipeInstances.push(load<CountingPipe>(1));
-            }
-            embeddedViewEnd();
-          }
-        }
-        containerRefreshEnd();
-      }
-
-      const pipeInstances: CountingPipe[] = [];
-      renderToHtml(Template, {}, null, defs, rendererFactory2);
-      expect(pipeInstances.length).toEqual(4);
-      expect(pipeInstances[0]).toBeAnInstanceOf(CountingPipe);
-      expect(pipeInstances[1]).toBe(pipeInstances[0]);
-      expect(pipeInstances[2]).toBe(pipeInstances[0]);
-      expect(pipeInstances[3]).toBe(pipeInstances[0]);
-    });
   });
 
   describe('impure', () => {