Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ivy): implement pipes #22254

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 6 additions & 3 deletions packages/core/src/render3/definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,10 @@ export const defineDirective = defineComponent as<T>(directiveDefinition: Direct
* @param pure Whether the pipe is pure.
*/
export function definePipe<T>(
{type, factory, pure}: {type: Type<T>, factory: () => PipeTransform, pure?: boolean}):
PipeDef<T> {
throw new Error('TODO: implement!');
{type, factory, pure}: {type: Type<T>, factory: () => T, pure?: boolean}): PipeDef<T> {
return <PipeDef<T>>{
n: factory,
pure: pure !== false,
onDestroy: type.prototype.ngOnDestroy || null
};
}
8 changes: 6 additions & 2 deletions packages/core/src/render3/instructions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {assertNodeType} from './node_assert';
import {appendChild, insertChild, insertView, appendProjectedNode, removeView, canInsertNativeNode} from './node_manipulation';
import {matchingSelectorIndex} from './node_selector_matcher';
import {ComponentDef, ComponentTemplate, ComponentType, DirectiveDef, DirectiveType} from './interfaces/definition';
import {RElement, RText, Renderer3, RendererFactory3, ProceduralRenderer3, RendererStyleFlags3, isProceduralRenderer} from './interfaces/renderer';
import {RElement, RText, Renderer3, RendererFactory3, ProceduralRenderer3, ObjectOrientedRenderer3, RendererStyleFlags3, isProceduralRenderer} from './interfaces/renderer';
import {isDifferent, stringify} from './util';
import {executeHooks, executeContentHooks, queueLifecycleHooks, queueInitHooks, executeInitHooks} from './hooks';
import {ViewRef} from './view_ref';
Expand Down Expand Up @@ -117,7 +117,7 @@ export function getCreationMode(): boolean {
}

/**
* An array of nodes (text, element, container, etc), their bindings, and
* An array of nodes (text, element, container, etc), pipes, their bindings, and
* any local variables that need to be stored between invocations.
*/
let data: any[];
Expand Down Expand Up @@ -1769,6 +1769,10 @@ export function bindingUpdated4(exp1: any, exp2: any, exp3: any, exp4: any): boo
return bindingUpdated2(exp3, exp4) || different;
}

export function getTView(): TView {
return currentView.tView;
}

export function getDirectiveInstance<T>(instanceOrArray: T | [T]): T {
// Directives with content queries store an array in data[directiveIndex]
// with the instance as the first index
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/render3/interfaces/definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ export interface PipeDef<T> {
* NOTE: this property is short (1 char) because it is used in
* component templates which is sensitive to size.
*/
n: () => PipeTransform;
n: () => T;

/**
* Whether or not the pipe is pure.
Expand Down
8 changes: 4 additions & 4 deletions packages/core/src/render3/interfaces/view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import {LContainer} from './container';
import {ComponentTemplate, DirectiveDef} from './definition';
import {ComponentTemplate, DirectiveDef, PipeDef} from './definition';
import {LElementNode, LViewNode, TNode} from './node';
import {LQueries} from './query';
import {Renderer3} from './renderer';
Expand Down Expand Up @@ -324,11 +324,11 @@ export const enum LifecycleStage {
* Static data that corresponds to the instance-specific data array on an LView.
*
* Each node's static data is stored in tData at the same index that it's stored
* in the data array. Each directive's definition is stored here at the same index
* as its directive instance in the data array. Any nodes that do not have static
* in the data array. Each directive/pipe's definition is stored here at the same index
* as its directive/pipe instance in the data array. Any nodes that do not have static
* data store a null value in tData to avoid a sparse array.
*/
export type TData = (TNode | DirectiveDef<any>| null)[];
export type TData = (TNode | DirectiveDef<any>| PipeDef<any>| null)[];

// Note: This hack is necessary so we don't erroneously get a circular dependency
// failure based on types.
Expand Down
47 changes: 38 additions & 9 deletions packages/core/src/render3/pipe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,32 @@
* found in the LICENSE file at https://angular.io/license
*/

import {PipeTransform} from '../change_detection/pipe_transform';

import {getTView, load, store} from './instructions';
import {PipeDef} from './interfaces/definition';
import {pureFunction1, pureFunction2, pureFunction3, pureFunction4, pureFunctionV} from './pure_function';


/**
* Create a pipe.
*
* @param index Pipe index where the pipe will be stored.
* @param pipeDef Pipe definition object for registering life cycle hooks.
* @param pipe A Pipe instance.
* @param firstInstance (optional) The first instance of the pipe that can be reused for pure pipes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of passing firstInstance as a Pipe we could pass it as an index which would greatly simplify the work which compiler has to do.

/cc @chuckjaz

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either is fine.

Storing it in a temporary that is not closure captured is simple to generate and is relatively efficient.

Passing in an index is easier. If it is already in an index then the index of the first one is an easy thing to track.

We should do whatever is consistent for the API of the instructions and the most efficient at runtime. The compiler can easily generate either.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using index was my first idea, but it wouldn't work with embedded views, and it should.

* @returns T the instance of the pipe.
*/
export function pipe<T>(index: number, pipeDef: PipeDef<T>, pipe: T): void {
throw new Error('TODO: implement!');
export function pipe<T>(index: number, pipeDef: PipeDef<T>, firstInstance?: T): T {
const tView = getTView();
if (tView.firstTemplatePass) {
tView.data[index] = pipeDef;
if (pipeDef.onDestroy != null) {
(tView.destroyHooks || (tView.destroyHooks = [])).push(index, pipeDef.onDestroy);
}
}
const pipeInstance = pipeDef.pure && firstInstance ? firstInstance : pipeDef.n();
store(index, pipeInstance);
return pipeInstance;
}

/**
Expand All @@ -29,7 +44,9 @@ export function pipe<T>(index: number, pipeDef: PipeDef<T>, pipe: T): void {
* @param v1 1st argument to {@link PipeTransform#transform}.
*/
export function pipeBind1(index: number, v1: any): any {
throw new Error('TODO: implement!');
const pipeInstance = load<PipeTransform>(index);
return isPure(index) ? pureFunction1(pipeInstance.transform, v1, pipeInstance) :
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an asymmetry between args in pure and non-pure case. They should be the same. Maybe have isPure && unwrap(x) as a way of turning pure with wrap into non-pure or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They can't be the same.

In the pure case, pureFunction1 will check the bindings and then, if some input has changed, it will unwrap the values and call transform.
In the impure case, transform can be called immediately with unwrapped values.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we have removed unwrap can we make them symmetric?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mhevery Can you please clarify what you have in mind?

pipeInstance.transform(v1);
}

/**
Expand All @@ -43,7 +60,9 @@ export function pipeBind1(index: number, v1: any): any {
* @param v2 2nd argument to {@link PipeTransform#transform}.
*/
export function pipeBind2(index: number, v1: any, v2: any): any {
throw new Error('TODO: implement!');
const pipeInstance = load<PipeTransform>(index);
return isPure(index) ? pureFunction2(pipeInstance.transform, v1, v2, pipeInstance) :
pipeInstance.transform(v1, v2);
}

/**
Expand All @@ -58,7 +77,9 @@ export function pipeBind2(index: number, v1: any, v2: any): any {
* @param v3 4rd argument to {@link PipeTransform#transform}.
*/
export function pipeBind3(index: number, v1: any, v2: any, v3: any): any {
throw new Error('TODO: implement!');
const pipeInstance = load<PipeTransform>(index);
return isPure(index) ? pureFunction3(pipeInstance.transform.bind(pipeInstance), v1, v2, v3) :
pipeInstance.transform(v1, v2, v3);
}

/**
Expand All @@ -74,7 +95,9 @@ export function pipeBind3(index: number, v1: any, v2: any, v3: any): any {
* @param v4 4th argument to {@link PipeTransform#transform}.
*/
export function pipeBind4(index: number, v1: any, v2: any, v3: any, v4: any): any {
throw new Error('TODO: implement!');
const pipeInstance = load<PipeTransform>(index);
return isPure(index) ? pureFunction4(pipeInstance.transform, v1, v2, v3, v4, pipeInstance) :
pipeInstance.transform(v1, v2, v3, v4);
}

/**
Expand All @@ -87,5 +110,11 @@ export function pipeBind4(index: number, v1: any, v2: any, v3: any, v4: any): an
* @param values Array of arguments to pass to {@link PipeTransform#transform} method.
*/
export function pipeBindV(index: number, values: any[]): any {
throw new Error('TODO: implement!');
}
const pipeInstance = load<PipeTransform>(index);
return isPure(index) ? pureFunctionV(pipeInstance.transform, values, pipeInstance) :
pipeInstance.transform.apply(pipeInstance, values);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pipeInstance.transform(pipeInstance, ...values); ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spread operator would be transpiled into a for loop, which doesn't seem to be a good idea.

}

function isPure(index: number): boolean {
return (<PipeDef<any>>getTView().data[index]).pure;
}
63 changes: 41 additions & 22 deletions packages/core/src/render3/pure_function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ import {bindingUpdated, bindingUpdated2, bindingUpdated4, checkAndUpdateBinding,
* @param pureFn Function that returns a value
* @returns value
*/
export function pureFunction0<T>(pureFn: () => T): T {
return getCreationMode() ? checkAndUpdateBinding(pureFn()) : consumeBinding();
export function pureFunction0<T>(pureFn: () => T, thisArg?: any): T {
return getCreationMode() ? checkAndUpdateBinding(thisArg ? pureFn.call(thisArg) : pureFn()) :
consumeBinding();
}

/**
Expand All @@ -29,8 +30,10 @@ export function pureFunction0<T>(pureFn: () => T): T {
* @param exp Updated expression value
* @returns Updated value
*/
export function pureFunction1(pureFn: (v: any) => any, exp: any): any {
return bindingUpdated(exp) ? checkAndUpdateBinding(pureFn(exp)) : consumeBinding();
export function pureFunction1(pureFn: (v: any) => any, exp: any, thisArg?: any): any {
return bindingUpdated(exp) ?
checkAndUpdateBinding(thisArg ? pureFn.call(thisArg, exp) : pureFn(exp)) :
consumeBinding();
}

/**
Expand All @@ -42,8 +45,11 @@ export function pureFunction1(pureFn: (v: any) => any, exp: any): any {
* @param exp2
* @returns Updated value
*/
export function pureFunction2(pureFn: (v1: any, v2: any) => any, exp1: any, exp2: any): any {
return bindingUpdated2(exp1, exp2) ? checkAndUpdateBinding(pureFn(exp1, exp2)) : consumeBinding();
export function pureFunction2(
pureFn: (v1: any, v2: any) => any, exp1: any, exp2: any, thisArg?: any): any {
return bindingUpdated2(exp1, exp2) ?
checkAndUpdateBinding(thisArg ? pureFn.call(thisArg, exp1, exp2) : pureFn(exp1, exp2)) :
consumeBinding();
}

/**
Expand All @@ -57,10 +63,13 @@ export function pureFunction2(pureFn: (v1: any, v2: any) => any, exp1: any, exp2
* @returns Updated value
*/
export function pureFunction3(
pureFn: (v1: any, v2: any, v3: any) => any, exp1: any, exp2: any, exp3: any): any {
pureFn: (v1: any, v2: any, v3: any) => any, exp1: any, exp2: any, exp3: any,
thisArg?: any): any {
const different = bindingUpdated2(exp1, exp2);
return bindingUpdated(exp3) || different ? checkAndUpdateBinding(pureFn(exp1, exp2, exp3)) :
consumeBinding();
return bindingUpdated(exp3) || different ?
checkAndUpdateBinding(
thisArg ? pureFn.call(thisArg, exp1, exp2, exp3) : pureFn(exp1, exp2, exp3)) :
consumeBinding();
}

/**
Expand All @@ -75,10 +84,11 @@ export function pureFunction3(
* @returns Updated value
*/
export function pureFunction4(
pureFn: (v1: any, v2: any, v3: any, v4: any) => any, exp1: any, exp2: any, exp3: any,
exp4: any): any {
pureFn: (v1: any, v2: any, v3: any, v4: any) => any, exp1: any, exp2: any, exp3: any, exp4: any,
thisArg?: any): any {
return bindingUpdated4(exp1, exp2, exp3, exp4) ?
checkAndUpdateBinding(pureFn(exp1, exp2, exp3, exp4)) :
checkAndUpdateBinding(
thisArg ? pureFn.call(thisArg, exp1, exp2, exp3, exp4) : pureFn(exp1, exp2, exp3, exp4)) :
consumeBinding();
}

Expand All @@ -96,10 +106,12 @@ export function pureFunction4(
*/
export function pureFunction5(
pureFn: (v1: any, v2: any, v3: any, v4: any, v5: any) => any, exp1: any, exp2: any, exp3: any,
exp4: any, exp5: any): any {
exp4: any, exp5: any, thisArg?: any): any {
const different = bindingUpdated4(exp1, exp2, exp3, exp4);
return bindingUpdated(exp5) || different ?
checkAndUpdateBinding(pureFn(exp1, exp2, exp3, exp4, exp5)) :
checkAndUpdateBinding(
thisArg ? pureFn.call(thisArg, exp1, exp2, exp3, exp4, exp5) :
pureFn(exp1, exp2, exp3, exp4, exp5)) :
consumeBinding();
}

Expand All @@ -118,10 +130,12 @@ export function pureFunction5(
*/
export function pureFunction6(
pureFn: (v1: any, v2: any, v3: any, v4: any, v5: any, v6: any) => any, exp1: any, exp2: any,
exp3: any, exp4: any, exp5: any, exp6: any): any {
exp3: any, exp4: any, exp5: any, exp6: any, thisArg?: any): any {
const different = bindingUpdated4(exp1, exp2, exp3, exp4);
return bindingUpdated2(exp5, exp6) || different ?
checkAndUpdateBinding(pureFn(exp1, exp2, exp3, exp4, exp5, exp6)) :
checkAndUpdateBinding(
thisArg ? pureFn.call(thisArg, exp1, exp2, exp3, exp4, exp5, exp6) :
pureFn(exp1, exp2, exp3, exp4, exp5, exp6)) :
consumeBinding();
}

Expand All @@ -141,11 +155,13 @@ export function pureFunction6(
*/
export function pureFunction7(
pureFn: (v1: any, v2: any, v3: any, v4: any, v5: any, v6: any, v7: any) => any, exp1: any,
exp2: any, exp3: any, exp4: any, exp5: any, exp6: any, exp7: any): any {
exp2: any, exp3: any, exp4: any, exp5: any, exp6: any, exp7: any, thisArg?: any): any {
let different = bindingUpdated4(exp1, exp2, exp3, exp4);
different = bindingUpdated2(exp5, exp6) || different;
return bindingUpdated(exp7) || different ?
checkAndUpdateBinding(pureFn(exp1, exp2, exp3, exp4, exp5, exp6, exp7)) :
checkAndUpdateBinding(
thisArg ? pureFn.call(thisArg, exp1, exp2, exp3, exp4, exp5, exp6, exp7) :
pureFn(exp1, exp2, exp3, exp4, exp5, exp6, exp7)) :
consumeBinding();
}

Expand All @@ -166,10 +182,13 @@ export function pureFunction7(
*/
export function pureFunction8(
pureFn: (v1: any, v2: any, v3: any, v4: any, v5: any, v6: any, v7: any, v8: any) => any,
exp1: any, exp2: any, exp3: any, exp4: any, exp5: any, exp6: any, exp7: any, exp8: any): any {
exp1: any, exp2: any, exp3: any, exp4: any, exp5: any, exp6: any, exp7: any, exp8: any,
thisArg?: any): any {
const different = bindingUpdated4(exp1, exp2, exp3, exp4);
return bindingUpdated4(exp5, exp6, exp7, exp8) || different ?
checkAndUpdateBinding(pureFn(exp1, exp2, exp3, exp4, exp5, exp6, exp7, exp8)) :
checkAndUpdateBinding(
thisArg ? pureFn.call(thisArg, exp1, exp2, exp3, exp4, exp5, exp6, exp7, exp8) :
pureFn(exp1, exp2, exp3, exp4, exp5, exp6, exp7, exp8)) :
consumeBinding();
}

Expand All @@ -184,11 +203,11 @@ export function pureFunction8(
* @param exp An array of binding values
* @returns Updated value
*/
export function pureFunctionV(pureFn: (...v: any[]) => any, exps: any[]): any {
export function pureFunctionV(pureFn: (...v: any[]) => any, exps: any[], thisArg?: any): any {
let different = false;

for (let i = 0; i < exps.length; i++) {
bindingUpdated(exps[i]) && (different = true);
}
return different ? checkAndUpdateBinding(pureFn.apply(null, exps)) : consumeBinding();
return different ? checkAndUpdateBinding(pureFn.apply(thisArg, exps)) : consumeBinding();
}