Skip to content

Commit 3f45682

Browse files
crisbetoalxhub
authored andcommitted
refactor(compiler): account for comments when resolving implicit deferred triggers (angular#52449)
Adds some logic to skip over comments when resolving implicit `@defer` block triggers. This currently isn't a problem since we don't capture comments by default, but it may come up if we start capturing comments. PR Close angular#52449
1 parent 2aaddd3 commit 3f45682

File tree

2 files changed

+44
-10
lines changed

2 files changed

+44
-10
lines changed

packages/compiler/src/render3/view/t2_binder.ts

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import {AST, BindingPipe, ImplicitReceiver, PropertyRead, PropertyWrite, RecursiveAstVisitor, SafePropertyRead} from '../../expression_parser/ast';
1010
import {SelectorMatcher} from '../../selector';
11-
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';
11+
import {BoundAttribute, BoundEvent, BoundText, Comment, 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';
1212

1313
import {BoundTarget, DirectiveMeta, ReferenceTarget, ScopedNode, Target, TargetBinder} from './t2_api';
1414
import {createCssSelector} from './template';
@@ -798,14 +798,29 @@ export class R3BoundTarget<DirectiveT extends DirectiveMeta> implements BoundTar
798798
const name = trigger.reference;
799799

800800
if (name === null) {
801-
const children = block.placeholder ? block.placeholder.children : null;
802-
803-
// If the trigger doesn't have a reference, it is inferred as the root element node of the
804-
// placeholder, if it only has one root node. Otherwise it's ambiguous so we don't
805-
// attempt to resolve further.
806-
return children !== null && children.length === 1 && children[0] instanceof Element ?
807-
children[0] :
808-
null;
801+
let trigger: Element|null = null;
802+
803+
if (block.placeholder !== null) {
804+
for (const child of block.placeholder.children) {
805+
// Skip over comment nodes. Currently by default the template parser doesn't capture
806+
// comments, but we have a safeguard here just in case since it can be enabled.
807+
if (child instanceof Comment) {
808+
continue;
809+
}
810+
811+
// We can only infer the trigger if there's one root element node. Any other
812+
// nodes at the root make it so that we can't infer the trigger anymore.
813+
if (trigger !== null) {
814+
return null;
815+
}
816+
817+
if (child instanceof Element) {
818+
trigger = child;
819+
}
820+
}
821+
}
822+
823+
return trigger;
809824
}
810825

811826
const outsideRef = this.findEntityInScope(block, name);
@@ -821,7 +836,7 @@ export class R3BoundTarget<DirectiveT extends DirectiveMeta> implements BoundTar
821836
}
822837

823838
// If the trigger couldn't be found in the main block, check the
824-
// placeholder block which is shown before the main block has loaded.
839+
// placeholder block which is shown before the main block has loaded.
825840
if (block.placeholder !== null) {
826841
const refInPlaceholder = this.findEntityInScope(block.placeholder, name);
827842
const targetInPlaceholder =

packages/compiler/test/render3/view/binding_spec.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,25 @@ describe('t2 binding', () => {
535535
expect(triggerEl?.name).toBe('button');
536536
});
537537

538+
it('should identify an implicit trigger inside the placeholder block with comments', () => {
539+
const template = parseTemplate(
540+
`
541+
@defer (on viewport) {
542+
main
543+
} @placeholder {
544+
<!-- before -->
545+
<button #trigger></button>
546+
<!-- after -->
547+
}
548+
`,
549+
'');
550+
const binder = new R3TargetBinder(makeSelectorMatcher());
551+
const bound = binder.bind({template: template.nodes});
552+
const block = Array.from(bound.getDeferBlocks())[0];
553+
const triggerEl = bound.getDeferredTriggerTarget(block, block.triggers.viewport!);
554+
expect(triggerEl?.name).toBe('button');
555+
});
556+
538557
it('should not identify an implicit trigger if the placeholder has multiple root nodes', () => {
539558
const template = parseTemplate(
540559
`

0 commit comments

Comments
 (0)