feat(ivy): add canonical example of a pipe. #21834

mhevery · 2018-01-27T21:12:54Z

No description provided.

pkozlowski-opensource · 2018-01-29T10:14:36Z

packages/core/src/render3/pipe.ts

+ * @param v1 1st argument to {@link PipeTransform#transform}.
+ */
+export function pipeBind1(index: number, v1: any): any {
+  throw new Error('TODO: implement!');


Those functions will be very similar to the existing b0...bV (logic to compare arguments, memorize those etc.). I wanted to refactor those methods so split them up into arguments checking / memorization before - maybe this is a good occasion?

Yes we should do this. Also chuck makes a good point that most of the code gen is in form of "{{exp}}"' which translates to b1('', exp, ''). So we should probably have it generate b1(exp)and than have separate instruction to add wrap like sow('', b1(exp),'')` and only generate the wrap if needed.

pkozlowski-opensource · 2018-01-29T10:46:47Z

One things I'm missing here is an example showing lifecycle hook(s) on pipes - if I'm not mistaken we can at least have OnDestroy.

Other than this LGTM as a general idea.
We will see more details when the runtime part gets implemented.

pkozlowski-opensource

Not sure if this is WIP and you are also planning to add:

runtime impl
lifecycle hooks
in this or a separate PR.

As a very minimum this PR needs re-format - lint is failing

mhevery · 2018-01-29T18:01:12Z

I was not planning on doing any implementation since this PR was to unblock @chuckjaz on complier implementation. Perhaps you or someone from Amadeus can take on the implementation?

I will fix:

fix formating
Add Lifecycle hooks.

mhevery · 2018-01-29T23:07:41Z

@pkozlowski-opensource please have another look.

kara

Mostly typos

kara · 2018-01-30T18:24:07Z

packages/core/src/metadata/directives.ts

+   * Name of the pipe.
+   *
+   * The pipe name is used in template bindings. For example if a pipe is named
+   * `myPipe` than it would be used in the template binding expression like


Typo: than -> then

kara · 2018-01-30T18:24:32Z

packages/core/src/metadata/directives.ts

+  /**
+   * If Pipe is pure (its output depends only on its input.)
+   *
+   * Normally pipe's `translate` method is invoked on each changed detection


Typo: changed -> change

kara · 2018-01-30T18:24:41Z

packages/core/src/metadata/directives.ts

+   *
+   * Normally pipe's `translate` method is invoked on each changed 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, than declaring a pipe


Typo: than -> then

kara · 2018-01-30T18:25:03Z

packages/core/src/metadata/directives.ts

  pure?: boolean;
 }

 /**
 * Pipe decorator and metadata.
 *
+ * Use the `@Pipe` annotation to declare the a given class is a pipe. A pipe


Typo: declare the a -> declare that a

kara · 2018-01-30T18:25:43Z

packages/core/src/render3/definition.ts

+ * ```
+ * class MyPipe implements PipeTransform {
+ *   // Generated by Angular Template Compiler
+ *   static ngPipDef = definePipe({


Typo: ngPipDef -> ngPipeDef

kara · 2018-01-30T18:26:58Z

packages/core/src/render3/interfaces/definition.ts

+  n: () => PipeTransform;
+
+  /**
+   * Wether or not the pipe is pure.


Typo: Wether -> Whether

kara · 2018-01-30T18:33:15Z

packages/core/test/render3/compiler_canonical_spec.ts

+      // NORMATIVE
+      static ngPipeDef = r3.definePipe({
+        type: MyPurePipe,
+        factory: function MyPipe_Factory() { return new MyPipe(); },


I think this should be

factory: function MyPurePipe_Factory() { return new MyPurePipe(); }

All concerns addressed

kara · 2018-01-30T19:28:56Z

packages/core/src/render3/pipe.ts

+ *
+ * @param index Pipe index where the pipe was stored on creation.
+ * @param v1 1st argument to {@link PipeTransform#transform}.
+ * @param v2 1st argument to {@link PipeTransform#transform}.


I think all of these in this file say "1st argument". This one should be 2nd, no? Same with others below

kara · 2018-01-30T19:29:21Z

packages/core/test/render3/compiler_canonical_spec.ts

+      // NORMATIVE
+      static ngPipeDef = r3.definePipe({
+        type: MyPurePipe,
+        factory: function MyPurePipe_Factory() { return new MyPipe(); },


Should be new MyPurePipe();, right?

kara

LGTM

ngbot · 2018-01-31T18:49:50Z

Hi @mhevery! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

PR Close angular#21834

angular-automatic-lock-bot · 2019-09-13T07:52:08Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

mhevery added action: review The PR is still awaiting reviews from at least one requested reviewer target: major This PR is targeted for the next major release labels Jan 27, 2018

mhevery requested review from chuckjaz, pkozlowski-opensource and kara January 27, 2018 21:12

googlebot added the cla: yes label Jan 27, 2018

pkozlowski-opensource reviewed Jan 29, 2018

View reviewed changes

pkozlowski-opensource previously requested changes Jan 29, 2018

View reviewed changes

jasonaden added the comp: ivy label Jan 29, 2018

mhevery force-pushed the ivy_pipes branch from 6cd4026 to d0756c1 Compare January 29, 2018 23:09

kara reviewed Jan 30, 2018

View reviewed changes

kara approved these changes Jan 30, 2018

View reviewed changes

mhevery force-pushed the ivy_pipes branch from 2c86077 to c388b5e Compare January 31, 2018 20:40

feat(ivy): add canonical example of a pipe.

c534396

mhevery force-pushed the ivy_pipes branch from c388b5e to c534396 Compare January 31, 2018 20:46

mhevery added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jan 31, 2018

alxhub closed this in 743d8bc Feb 2, 2018

mhevery deleted the ivy_pipes branch February 14, 2018 23:01

jbogarthyde pushed a commit to jbogarthyde/angular that referenced this pull request Feb 23, 2018

feat(ivy): add canonical example of a pipe. (angular#21834)

69c9ab1

PR Close angular#21834

leo6104 pushed a commit to leo6104/angular that referenced this pull request Mar 25, 2018

feat(ivy): add canonical example of a pipe. (angular#21834)

15d0cff

PR Close angular#21834

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ivy): add canonical example of a pipe. #21834

feat(ivy): add canonical example of a pipe. #21834

mhevery commented Jan 27, 2018

pkozlowski-opensource Jan 29, 2018

mhevery Jan 29, 2018

pkozlowski-opensource commented Jan 29, 2018

pkozlowski-opensource left a comment

mhevery commented Jan 29, 2018 •

edited

mhevery commented Jan 29, 2018

kara left a comment

kara Jan 30, 2018

mhevery Jan 30, 2018

kara Jan 30, 2018

mhevery Jan 30, 2018

kara Jan 30, 2018

mhevery Jan 30, 2018

kara Jan 30, 2018

mhevery Jan 30, 2018

kara Jan 30, 2018

mhevery Jan 30, 2018

kara Jan 30, 2018

mhevery Jan 30, 2018

kara Jan 30, 2018

mhevery Jan 30, 2018

kara Jan 30, 2018 •

edited

mhevery Jan 30, 2018

kara Jan 30, 2018

mhevery Jan 30, 2018

kara left a comment

ngbot bot commented Jan 31, 2018

angular-automatic-lock-bot bot commented Sep 13, 2019

feat(ivy): add canonical example of a pipe. #21834

feat(ivy): add canonical example of a pipe. #21834

Conversation

mhevery commented Jan 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pkozlowski-opensource commented Jan 29, 2018

pkozlowski-opensource left a comment

Choose a reason for hiding this comment

mhevery commented Jan 29, 2018 • edited

mhevery commented Jan 29, 2018

kara left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kara Jan 30, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kara left a comment

Choose a reason for hiding this comment

ngbot bot commented Jan 31, 2018

angular-automatic-lock-bot bot commented Sep 13, 2019

mhevery commented Jan 29, 2018 •

edited

kara Jan 30, 2018 •

edited