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.

Sign up for GitHub

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

Jump to bottom

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

Closed
wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 4 additions & 1 deletion packages/compiler/src/render3/r3_pipe_compiler.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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