Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix(compiler): detect pipes in ICUs in template binder (#38810)
Recent work on compiler internals in #38539 led to an unexpected failure,
where a pipe used exclusively inside of an ICU would no longer be
emitted into the compilation output. This caused runtime errors due to
missing pipes.

The issue occurred because the change in #38539 would determine the set
of used pipes up-front, independent from the template compilation using
the `R3TargetBinder`. However, `R3TargetBinder` did not consider
expressions within ICUs, so any pipe usages within those expressions
would not be detected. This fix unblocks #38539 and also concerns
upcoming linker work, given that prelink compilations would not go
through full template compilation but only `R3TargetBinder`.

PR Close #38810
  • Loading branch information
JoostK authored and AndrewKushnir committed Sep 11, 2020
1 parent 8096c63 commit ec2dbe7
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 1 deletion.
5 changes: 4 additions & 1 deletion packages/compiler/src/render3/view/t2_binder.ts
Expand Up @@ -434,7 +434,10 @@ class TemplateBinder extends RecursiveAstVisitor implements Visitor {
visitText(text: Text) {}
visitContent(content: Content) {}
visitTextAttribute(attribute: TextAttribute) {}
visitIcu(icu: Icu): void {}
visitIcu(icu: Icu): void {
Object.keys(icu.vars).forEach(key => icu.vars[key].visit(this));
Object.keys(icu.placeholders).forEach(key => icu.placeholders[key].visit(this));
}

// The remaining visitors are concerned with processing AST expressions within template bindings

Expand Down
34 changes: 34 additions & 0 deletions packages/compiler/test/render3/view/binding_spec.ts
Expand Up @@ -194,4 +194,38 @@ describe('t2 binding', () => {
expect(consumer).toEqual(el);
});
});

describe('used pipes', () => {
it('should record pipes used in interpolations', () => {
const template = parseTemplate('{{value|date}}', '', {});
const binder = new R3TargetBinder(makeSelectorMatcher());
const res = binder.bind({template: template.nodes});
expect(res.getUsedPipes()).toEqual(['date']);
});

it('should record pipes used in bound attributes', () => {
const template = parseTemplate('<person [age]="age|number"></person>', '', {});
const binder = new R3TargetBinder(makeSelectorMatcher());
const res = binder.bind({template: template.nodes});
expect(res.getUsedPipes()).toEqual(['number']);
});

it('should record pipes used in bound template attributes', () => {
const template = parseTemplate('<ng-template [ngIf]="obs|async"></ng-template>', '', {});
const binder = new R3TargetBinder(makeSelectorMatcher());
const res = binder.bind({template: template.nodes});
expect(res.getUsedPipes()).toEqual(['async']);
});

it('should record pipes used in ICUs', () => {
const template = parseTemplate(
`<span i18n>{count|number, plural,
=1 { {{value|date}} }
}</span>`,
'', {});
const binder = new R3TargetBinder(makeSelectorMatcher());
const res = binder.bind({template: template.nodes});
expect(res.getUsedPipes()).toEqual(['number', 'date']);
});
});
});

0 comments on commit ec2dbe7

Please sign in to comment.