Skip to content

Commit

Permalink
refactor(ivy): generate pipe names instead of defs (#23104)
Browse files Browse the repository at this point in the history 
PR Close #23104
  • Loading branch information
@kara @alxhub
kara authored and alxhub committed Apr 2, 2018
1 parent 43d6202 commit 85d3b59
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 76 deletions.
5 changes: 4 additions & 1 deletion packages/compiler/src/render3/r3_pipe_compiler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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});

Expand Down
50 changes: 34 additions & 16 deletions packages/compiler/src/render3/r3_view_compiler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
pipe.type.reference, DefinitionKind.Pipe, outputCtx, /* forceShared */ true);
this.addPipeDependency(pipe);
this._creationMode.push(
o.importExpr(R3.pipe)
.callFn([
o.literal(slot), pipeDefinition, pipeDefinition.callMethod(R3.NEW_METHOD, [])
])
.toStmt());
o.importExpr(R3.pipe).callFn([o.literal(slot), o.literal(name)]).toStmt());
});
}

Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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);
if (slot == null) {
slot = this.allocateSlot();
this.pipeSlots.set(ast.name, slot);
}
const slot = this.allocateSlot();
const slotPseudoLocal = `PIPE:${slot}`;
const target = new PropertyRead(ast.span, new ImplicitReceiver(ast.span), slotPseudoLocal);
const bindingId = pipeBinding(ast.args);
Expand Down
23 changes: 15 additions & 8 deletions packages/compiler/test/render3/r3_compiler_compliance_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ describe('compiler compliance', () => {
template: function ChildComponent_Template(ctx: IDENT, cm: IDENT) {
if (cm) {
$r3$.ɵT(0, 'child-view');
}
}
});`;
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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(2, $MyPipe_ngPipeDef$, $MyPipe_ngPipeDef$.n());
$r3$.ɵPp(1, 'myPurePipe');
$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);
Expand Down
7 changes: 3 additions & 4 deletions packages/core/src/render3/pipe.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 firstInstance (optional) The first instance of the pipe that can be reused for pure pipes.
* @param pipeName The name of the pipe
* @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>;

Expand All @@ -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;
}
Expand Down
7 changes: 3 additions & 4 deletions packages/core/test/render3/compiler_canonical/pipes_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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), ''));
Expand All @@ -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), ''));
Expand Down
43 changes: 0 additions & 43 deletions packages/core/test/render3/pipe_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down

0 comments on commit 85d3b59

Please sign in to comment.