diff --git a/packages/compiler/src/render3/view/t2_api.ts b/packages/compiler/src/render3/view/t2_api.ts index 179bf72907e..4c5ca76fe79 100644 --- a/packages/compiler/src/render3/view/t2_api.ts +++ b/packages/compiler/src/render3/view/t2_api.ts @@ -20,6 +20,7 @@ import { ForLoopBlock, ForLoopBlockEmpty, IfBlockBranch, + LetDeclaration, Node, Reference, SwitchBlockCase, @@ -50,6 +51,9 @@ export type ReferenceTarget = | Element | Template; +/** Entity that is local to the template and defined within the template. */ +export type TemplateEntity = Reference | Variable | LetDeclaration; + /* * t2 is the replacement for the `TemplateDefinitionBuilder`. It handles the operations of * analyzing Angular templates, extracting semantic info, and ultimately producing a template @@ -200,7 +204,7 @@ export interface BoundTarget { * This is only defined for `AST` expressions that read or write to a property of an * `ImplicitReceiver`. */ - getExpressionTarget(expr: AST): Reference | Variable | null; + getExpressionTarget(expr: AST): TemplateEntity | null; /** * Given a particular `Reference` or `Variable`, get the `ScopedNode` which created it. @@ -208,7 +212,7 @@ export interface BoundTarget { * All `Variable`s are defined on node, so this will always return a value for a `Variable` * from the `Target`. Returns `null` otherwise. */ - getDefinitionNodeOfSymbol(symbol: Reference | Variable): ScopedNode | null; + getDefinitionNodeOfSymbol(symbol: TemplateEntity): ScopedNode | null; /** * Get the nesting level of a particular `ScopedNode`. @@ -222,7 +226,7 @@ export interface BoundTarget { * Get all `Reference`s and `Variables` visible within the given `ScopedNode` (or at the top * level, if `null` is passed). */ - getEntitiesInScope(node: ScopedNode | null): ReadonlySet; + getEntitiesInScope(node: ScopedNode | null): ReadonlySet; /** * Get a list of all the directives used by the target, diff --git a/packages/compiler/src/render3/view/t2_binder.ts b/packages/compiler/src/render3/view/t2_binder.ts index c209048e82b..0ba2681fc51 100644 --- a/packages/compiler/src/render3/view/t2_binder.ts +++ b/packages/compiler/src/render3/view/t2_binder.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ +import {TmplAstLetDeclaration} from '../../compiler'; import { AST, BindingPipe, @@ -14,6 +15,7 @@ import { PropertyWrite, RecursiveAstVisitor, SafePropertyRead, + ThisReceiver, } from '../../expression_parser/ast'; import {SelectorMatcher} from '../../selector'; import { @@ -56,6 +58,7 @@ import { ScopedNode, Target, TargetBinder, + TemplateEntity, } from './t2_api'; import {createCssSelectorFromNode} from './util'; @@ -125,7 +128,7 @@ class Scope implements Visitor { /** * Named members of the `Scope`, such as `Reference`s or `Variable`s. */ - readonly namedEntities = new Map(); + readonly namedEntities = new Map(); /** * Set of elements that belong to this scope. @@ -275,7 +278,7 @@ class Scope implements Visitor { } visitLetDeclaration(decl: LetDeclaration) { - // TODO(crisbeto): needs further integration + this.maybeDeclare(decl); } // Unused visitors. @@ -288,7 +291,7 @@ class Scope implements Visitor { visitDeferredTrigger(trigger: DeferredTrigger) {} visitUnknownBlock(block: UnknownBlock) {} - private maybeDeclare(thing: Reference | Variable) { + private maybeDeclare(thing: TemplateEntity) { // Declare something with a name, as long as that name isn't taken. if (!this.namedEntities.has(thing.name)) { this.namedEntities.set(thing.name, thing); @@ -300,7 +303,7 @@ class Scope implements Visitor { * * This can recurse into a parent `Scope` if it's available. */ - lookup(name: string): Reference | Variable | null { + lookup(name: string): TemplateEntity | null { if (this.namedEntities.has(name)) { // Found in the local scope. return this.namedEntities.get(name)!; @@ -541,10 +544,6 @@ class DirectiveBinder implements Visitor { content.children.forEach((child) => child.visit(this)); } - visitLetDeclaration(decl: LetDeclaration) { - // TODO(crisbeto): needs further integration - } - // Unused visitors. visitVariable(variable: Variable): void {} visitReference(reference: Reference): void {} @@ -557,6 +556,7 @@ class DirectiveBinder implements Visitor { visitIcu(icu: Icu): void {} visitDeferredTrigger(trigger: DeferredTrigger): void {} visitUnknownBlock(block: UnknownBlock) {} + visitLetDeclaration(decl: LetDeclaration) {} } /** @@ -572,8 +572,8 @@ class TemplateBinder extends RecursiveAstVisitor implements Visitor { private visitNode: (node: Node) => void; private constructor( - private bindings: Map, - private symbols: Map, + private bindings: Map, + private symbols: Map, private usedPipes: Set, private eagerPipes: Set, private deferBlocks: [DeferredBlock, Scope][], @@ -615,15 +615,15 @@ class TemplateBinder extends RecursiveAstVisitor implements Visitor { nodes: Node[], scope: Scope, ): { - expressions: Map; - symbols: Map; + expressions: Map; + symbols: Map; nestingLevel: Map; usedPipes: Set; eagerPipes: Set; deferBlocks: [DeferredBlock, Scope][]; } { - const expressions = new Map(); - const symbols = new Map(); + const expressions = new Map(); + const symbols = new Map(); const nestingLevel = new Map(); const usedPipes = new Set(); const eagerPipes = new Set(); @@ -803,8 +803,11 @@ class TemplateBinder extends RecursiveAstVisitor implements Visitor { } visitLetDeclaration(decl: LetDeclaration) { - // TODO(crisbeto): needs further integration decl.value.visit(this); + + if (this.rootNode !== null) { + this.symbols.set(decl, this.rootNode); + } } override visitPipe(ast: BindingPipe, context: any): any { @@ -858,7 +861,15 @@ class TemplateBinder extends RecursiveAstVisitor implements Visitor { // Check whether the name exists in the current scope. If so, map it. Otherwise, the name is // probably a property on the top-level component context. - let target = this.scope.lookup(name); + const target = this.scope.lookup(name); + + // It's not allowed to read template entities via `this`, however it previously worked by + // accident (see #55115). Since `@let` declarations are new, we can fix it from the beginning, + // whereas pre-existing template entities will be fixed in #55115. + if (target instanceof TmplAstLetDeclaration && ast.receiver instanceof ThisReceiver) { + return; + } + if (target !== null) { this.bindings.set(ast, target); } @@ -889,10 +900,10 @@ export class R3BoundTarget implements BoundTar BoundAttribute | BoundEvent | Reference | TextAttribute, {directive: DirectiveT; node: Element | Template} | Element | Template >, - private exprTargets: Map, - private symbols: Map, + private exprTargets: Map, + private symbols: Map, private nestingLevel: Map, - private scopedNodeEntities: Map>, + private scopedNodeEntities: Map>, private usedPipes: Set, private eagerPipes: Set, rawDeferred: [DeferredBlock, Scope][], @@ -901,7 +912,7 @@ export class R3BoundTarget implements BoundTar this.deferredScopes = new Map(rawDeferred); } - getEntitiesInScope(node: ScopedNode | null): ReadonlySet { + getEntitiesInScope(node: ScopedNode | null): ReadonlySet { return this.scopedNodeEntities.get(node) ?? new Set(); } @@ -919,11 +930,11 @@ export class R3BoundTarget implements BoundTar return this.bindings.get(binding) || null; } - getExpressionTarget(expr: AST): Reference | Variable | null { + getExpressionTarget(expr: AST): TemplateEntity | null { return this.exprTargets.get(expr) || null; } - getDefinitionNodeOfSymbol(symbol: Reference | Variable): ScopedNode | null { + getDefinitionNodeOfSymbol(symbol: TemplateEntity): ScopedNode | null { return this.symbols.get(symbol) || null; } @@ -1046,7 +1057,7 @@ export class R3BoundTarget implements BoundTar * @param rootNode Root node of the scope. * @param name Name of the entity. */ - private findEntityInScope(rootNode: ScopedNode, name: string): Reference | Variable | null { + private findEntityInScope(rootNode: ScopedNode, name: string): TemplateEntity | null { const entities = this.getEntitiesInScope(rootNode); for (const entity of entities) { @@ -1072,19 +1083,17 @@ export class R3BoundTarget implements BoundTar } } -function extractScopedNodeEntities( - rootScope: Scope, -): Map> { - const entityMap = new Map>(); +function extractScopedNodeEntities(rootScope: Scope): Map> { + const entityMap = new Map>(); - function extractScopeEntities(scope: Scope): Map { + function extractScopeEntities(scope: Scope): Map { if (entityMap.has(scope.rootNode)) { return entityMap.get(scope.rootNode)!; } const currentEntities = scope.namedEntities; - let entities: Map; + let entities: Map; if (scope.parentScope !== null) { entities = new Map([...extractScopeEntities(scope.parentScope), ...currentEntities]); } else { @@ -1104,7 +1113,7 @@ function extractScopedNodeEntities( extractScopeEntities(scope); } - const templateEntities = new Map>(); + const templateEntities = new Map>(); for (const [template, entities] of entityMap) { templateEntities.set(template, new Set(entities.values())); } diff --git a/packages/compiler/test/render3/view/binding_spec.ts b/packages/compiler/test/render3/view/binding_spec.ts index d6858825ca9..4f05b57397f 100644 --- a/packages/compiler/test/render3/view/binding_spec.ts +++ b/packages/compiler/test/render3/view/binding_spec.ts @@ -212,6 +212,126 @@ describe('t2 binding', () => { expect(elDirectives[0].name).toBe('Dir'); }); + it('should get @let declarations when resolving entities at the root', () => { + const template = parseTemplate( + ` + @let one = 1; + @let two = 2; + @let sum = one + two; + `, + '', + { + enableLetSyntax: true, + }, + ); + const binder = new R3TargetBinder(new SelectorMatcher()); + const res = binder.bind({template: template.nodes}); + const entities = Array.from(res.getEntitiesInScope(null)); + + expect(entities.map((entity) => entity.name)).toEqual(['one', 'two', 'sum']); + }); + + it('should scope @let declarations to their current view', () => { + const template = parseTemplate( + ` + @let one = 1; + + @if (true) { + @let two = 2; + } + + @if (true) { + @let three = 3; + } + `, + '', + { + enableLetSyntax: true, + }, + ); + const binder = new R3TargetBinder(new SelectorMatcher()); + const res = binder.bind({template: template.nodes}); + const rootEntities = Array.from(res.getEntitiesInScope(null)); + const firstBranchEntities = Array.from( + res.getEntitiesInScope((template.nodes[1] as a.IfBlock).branches[0]), + ); + const secondBranchEntities = Array.from( + res.getEntitiesInScope((template.nodes[2] as a.IfBlock).branches[0]), + ); + + expect(rootEntities.map((entity) => entity.name)).toEqual(['one']); + expect(firstBranchEntities.map((entity) => entity.name)).toEqual(['one', 'two']); + expect(secondBranchEntities.map((entity) => entity.name)).toEqual(['one', 'three']); + }); + + it('should resolve expressions to an @let declaration', () => { + const template = parseTemplate( + ` + @let value = 1; + {{value}} + `, + '', + { + enableLetSyntax: true, + }, + ); + const binder = new R3TargetBinder(new SelectorMatcher()); + const res = binder.bind({template: template.nodes}); + const interpolationWrapper = (template.nodes[1] as a.BoundText).value as e.ASTWithSource; + const propertyRead = (interpolationWrapper.ast as e.Interpolation).expressions[0]; + const target = res.getExpressionTarget(propertyRead); + + expect(target instanceof a.LetDeclaration).toBe(true); + expect((target as a.LetDeclaration)?.name).toBe('value'); + }); + + it('should not resolve a `this` access to a `@let` declaration', () => { + const template = parseTemplate( + ` + @let value = 1; + {{this.value}} + `, + '', + { + enableLetSyntax: true, + }, + ); + const binder = new R3TargetBinder(new SelectorMatcher()); + const res = binder.bind({template: template.nodes}); + const interpolationWrapper = (template.nodes[1] as a.BoundText).value as e.ASTWithSource; + const propertyRead = (interpolationWrapper.ast as e.Interpolation).expressions[0]; + const target = res.getExpressionTarget(propertyRead); + + expect(target).toBe(null); + }); + + it('should resolve the definition node of let declarations', () => { + const template = parseTemplate( + ` + @if (true) { + @let one = 1; + } + + @if (true) { + @let two = 2; + } + `, + '', + { + enableLetSyntax: true, + }, + ); + const binder = new R3TargetBinder(new SelectorMatcher()); + const res = binder.bind({template: template.nodes}); + const firstBranch = (template.nodes[0] as a.IfBlock).branches[0]; + const firstLet = firstBranch.children[0] as a.LetDeclaration; + const secondBranch = (template.nodes[1] as a.IfBlock).branches[0]; + const secondLet = secondBranch.children[0] as a.LetDeclaration; + + expect(res.getDefinitionNodeOfSymbol(firstLet)).toBe(firstBranch); + expect(res.getDefinitionNodeOfSymbol(secondLet)).toBe(secondBranch); + }); + describe('matching inputs to consuming directives', () => { it('should work for bound attributes', () => { const template = parseTemplate('
', '', {});