Skip to content

Commit

Permalink
fix(compiler): pipes used inside defer triggers not being picked up (a…
Browse files Browse the repository at this point in the history
…ngular#52071)

Fixes that the compiler wasn't picking up pipes used inside defer block triggers as dependencies. We had implemented the `visitDeferredTrigger` visitor method, but it wasn't being called, because we weren't going through the `visitAll` method of the deferred block. We don't use `visitAll`, because child nodes have to be processed differently than the connected blocks and triggers.

Fixes angular#52068.

PR Close angular#52071
  • Loading branch information
crisbeto authored and ChellappanRajan committed Jan 23, 2024
1 parent 08fca45 commit f2cc476
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 8 deletions.
Expand Up @@ -517,7 +517,7 @@ MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "17.0.0", version: "0.0.0-
@defer (when isVisible() && (isReady | testPipe)) {
Hello
}
`, isInline: true });
`, isInline: true, dependencies: [{ kind: "pipe", type: TestPipe, name: "testPipe" }] });
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{
type: Component,
args: [{
Expand Down
99 changes: 99 additions & 0 deletions packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
Expand Up @@ -9053,6 +9053,105 @@ function allTests(os: string) {
});
});

it('should detect pipe used in the `when` trigger as an eager dependency', () => {
env.write('test-pipe.ts', `
import { Pipe } from '@angular/core';
@Pipe({name: 'test', standalone: true})
export class TestPipe {
transform() {
return 1;
}
}
`);

env.write('/test.ts', `
import { Component } from '@angular/core';
import { TestPipe } from './test-pipe';
@Component({
selector: 'test-cmp',
standalone: true,
imports: [TestPipe],
template: '@defer (when 1 | test) { hello }',
})
export class TestCmp {
}
`);

env.driveMain();

const jsContents = env.getContents('test.js');

expect(jsContents).toContain('dependencies: [TestPipe]');
});

it('should detect pipe used in the `prefetch when` trigger as an eager dependency', () => {
env.write('test-pipe.ts', `
import { Pipe } from '@angular/core';
@Pipe({name: 'test', standalone: true})
export class TestPipe {
transform() {
return 1;
}
}
`);

env.write('/test.ts', `
import { Component } from '@angular/core';
import { TestPipe } from './test-pipe';
@Component({
selector: 'test-cmp',
standalone: true,
imports: [TestPipe],
template: '@defer (when 1 | test) { hello }',
})
export class TestCmp {
}
`);

env.driveMain();

const jsContents = env.getContents('test.js');

expect(jsContents).toContain('dependencies: [TestPipe]');
});

it('should detect pipe used both in a trigger and the deferred content as eager', () => {
env.write('test-pipe.ts', `
import { Pipe } from '@angular/core';
@Pipe({name: 'test', standalone: true})
export class TestPipe {
transform() {
return 1;
}
}
`);

env.write('/test.ts', `
import { Component } from '@angular/core';
import { TestPipe } from './test-pipe';
@Component({
selector: 'test-cmp',
standalone: true,
imports: [TestPipe],
template: '@defer (when 1 | test) { {{1 | test}} }',
})
export class TestCmp {
}
`);

env.driveMain();

const jsContents = env.getContents('test.js');

expect(jsContents).toContain('dependencies: [TestPipe]');
});

describe('setClassMetadataAsync', () => {
it('should generate setClassMetadataAsync for components with defer blocks', () => {
env.write('cmp-a.ts', `
Expand Down
11 changes: 4 additions & 7 deletions packages/compiler/src/render3/view/t2_binder.ts
Expand Up @@ -8,7 +8,7 @@

import {AST, BindingPipe, ImplicitReceiver, PropertyRead, PropertyWrite, RecursiveAstVisitor, SafePropertyRead} from '../../expression_parser/ast';
import {SelectorMatcher} from '../../selector';
import {BoundAttribute, BoundDeferredTrigger, BoundEvent, BoundText, Content, DeferredBlock, DeferredBlockError, DeferredBlockLoading, DeferredBlockPlaceholder, DeferredTrigger, Element, ForLoopBlock, ForLoopBlockEmpty, HoverDeferredTrigger, Icu, IfBlock, IfBlockBranch, InteractionDeferredTrigger, Node, Reference, SwitchBlock, SwitchBlockCase, Template, Text, TextAttribute, UnknownBlock, Variable, ViewportDeferredTrigger, Visitor} from '../r3_ast';
import {BoundAttribute, BoundEvent, BoundText, Content, DeferredBlock, DeferredBlockError, DeferredBlockLoading, DeferredBlockPlaceholder, DeferredTrigger, Element, ForLoopBlock, ForLoopBlockEmpty, HoverDeferredTrigger, Icu, IfBlock, IfBlockBranch, InteractionDeferredTrigger, Node, Reference, SwitchBlock, SwitchBlockCase, Template, Text, TextAttribute, UnknownBlock, Variable, ViewportDeferredTrigger, Visitor} from '../r3_ast';

import {BoundTarget, DirectiveMeta, ReferenceTarget, ScopedNode, Target, TargetBinder} from './t2_api';
import {createCssSelector} from './template';
Expand Down Expand Up @@ -593,6 +593,7 @@ class TemplateBinder extends RecursiveAstVisitor implements Visitor {
visitContent(content: Content) {}
visitTextAttribute(attribute: TextAttribute) {}
visitUnknownBlock(block: UnknownBlock) {}
visitDeferredTrigger(): 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));
Expand All @@ -612,17 +613,13 @@ class TemplateBinder extends RecursiveAstVisitor implements Visitor {
this.deferBlocks.add(deferred);
this.ingestScopedNode(deferred);

deferred.triggers.when?.value.visit(this);
deferred.prefetchTriggers.when?.value.visit(this);
deferred.placeholder && this.visitNode(deferred.placeholder);
deferred.loading && this.visitNode(deferred.loading);
deferred.error && this.visitNode(deferred.error);
}

visitDeferredTrigger(trigger: DeferredTrigger): void {
if (trigger instanceof BoundDeferredTrigger) {
trigger.value.visit(this);
}
}

visitDeferredBlockPlaceholder(block: DeferredBlockPlaceholder) {
this.ingestScopedNode(block);
}
Expand Down

0 comments on commit f2cc476

Please sign in to comment.