refactor(compiler): integrate let declarations into the template binder (angular#56199)

Integrates the let declarations into the template binder which will be used to power the scoping behavior of the new syntax. PR Close angular#56199

Author: crisbeto

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<DirectiveT> =
  | 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,15 +204,15 @@ export interface BoundTarget<DirectiveT extends DirectiveMeta> {
  * 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.
  *
  * 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<DirectiveT extends DirectiveMeta> {
  * 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<Reference | Variable>;
+ getEntitiesInScope(node: ScopedNode | null): ReadonlySet<TemplateEntity>;
 
  /**
  * Get a list of all the directives used by the target,

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<string, Reference | Variable>();
+ readonly namedEntities = new Map<string, TemplateEntity>();
 
  /**
  * 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<DirectiveT extends DirectiveMeta> 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<DirectiveT extends DirectiveMeta> 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<AST, Reference | Variable>,
- private symbols: Map<Reference | Variable, ScopedNode>,
+ private bindings: Map<AST, TemplateEntity>,
+ private symbols: Map<TemplateEntity, ScopedNode>,
  private usedPipes: Set<string>,
  private eagerPipes: Set<string>,
  private deferBlocks: [DeferredBlock, Scope][],
@@ -615,15 +615,15 @@ class TemplateBinder extends RecursiveAstVisitor implements Visitor {
  nodes: Node[],
  scope: Scope,
  ): {
- expressions: Map<AST, Reference | Variable>;
- symbols: Map<Variable | Reference, Template>;
+ expressions: Map<AST, TemplateEntity>;
+ symbols: Map<TemplateEntity, Template>;
  nestingLevel: Map<ScopedNode, number>;
  usedPipes: Set<string>;
  eagerPipes: Set<string>;
  deferBlocks: [DeferredBlock, Scope][];
  } {
- const expressions = new Map<AST, Reference | Variable>();
- const symbols = new Map<Variable | Reference, Template>();
+ const expressions = new Map<AST, TemplateEntity>();
+ const symbols = new Map<TemplateEntity, Template>();
  const nestingLevel = new Map<ScopedNode, number>();
  const usedPipes = new Set<string>();
  const eagerPipes = new Set<string>();
@@ -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<DirectiveT extends DirectiveMeta> implements BoundTar
  BoundAttribute | BoundEvent | Reference | TextAttribute,
  {directive: DirectiveT; node: Element | Template} | Element | Template
  >,
- private exprTargets: Map<AST, Reference | Variable>,
- private symbols: Map<Reference | Variable, Template>,
+ private exprTargets: Map<AST, TemplateEntity>,
+ private symbols: Map<TemplateEntity, Template>,
  private nestingLevel: Map<ScopedNode, number>,
- private scopedNodeEntities: Map<ScopedNode | null, ReadonlySet<Reference | Variable>>,
+ private scopedNodeEntities: Map<ScopedNode | null, ReadonlySet<TemplateEntity>>,
  private usedPipes: Set<string>,
  private eagerPipes: Set<string>,
  rawDeferred: [DeferredBlock, Scope][],
@@ -901,7 +912,7 @@ export class R3BoundTarget<DirectiveT extends DirectiveMeta> implements BoundTar
  this.deferredScopes = new Map(rawDeferred);
  }
 
- getEntitiesInScope(node: ScopedNode | null): ReadonlySet<Reference | Variable> {
+ getEntitiesInScope(node: ScopedNode | null): ReadonlySet<TemplateEntity> {
  return this.scopedNodeEntities.get(node) ?? new Set();
  }
 
@@ -919,11 +930,11 @@ export class R3BoundTarget<DirectiveT extends DirectiveMeta> 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<DirectiveT extends DirectiveMeta> 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<DirectiveT extends DirectiveMeta> implements BoundTar
  }
 }
 
-function extractScopedNodeEntities(
- rootScope: Scope,
-): Map<ScopedNode | null, Set<Reference | Variable>> {
- const entityMap = new Map<ScopedNode | null, Map<string, Reference | Variable>>();
+function extractScopedNodeEntities(rootScope: Scope): Map<ScopedNode | null, Set<TemplateEntity>> {
+ const entityMap = new Map<ScopedNode | null, Map<string, TemplateEntity>>();
 
- function extractScopeEntities(scope: Scope): Map<string, Reference | Variable> {
+ function extractScopeEntities(scope: Scope): Map<string, TemplateEntity> {
  if (entityMap.has(scope.rootNode)) {
  return entityMap.get(scope.rootNode)!;
  }
 
  const currentEntities = scope.namedEntities;
 
- let entities: Map<string, Reference | Variable>;
+ let entities: Map<string, TemplateEntity>;
  if (scope.parentScope !== null) {
  entities = new Map([...extractScopeEntities(scope.parentScope), ...currentEntities]);
  } else {
@@ -1104,7 +1113,7 @@ function extractScopedNodeEntities(
  extractScopeEntities(scope);
  }
 
- const templateEntities = new Map<ScopedNode | null, Set<Reference | Variable>>();
+ const templateEntities = new Map<ScopedNode | null, Set<TemplateEntity>>();
  for (const [template, entities] of entityMap) {
  templateEntities.set(template, new Set(entities.values()));
  }

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<DirectiveMeta[]>());
+ 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<DirectiveMeta[]>());
+ 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<DirectiveMeta[]>());
+ 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<DirectiveMeta[]>());
+ 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<DirectiveMeta[]>());
+ 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('<div hasInput [inputBinding]="myValue"></div>', '', {});