mirror of
https://github.com/angular/angular.git
synced 2026-09-14 13:54:52 +08:00
fix(compiler): narrow the type of the aliased if block expression (#51952)
Currently the TCB for aliased `if` blocks looks something like this:
```
// Markup: `@if (expr; as alias) { {{alias}} }
if (block.condition) {
var alias = block.condition;
"" + alias;
}
```
The problem with this approach is that the type of `alias` won't be narrowed. This is something that `NgIf` currently supports.
These changes resolve the issue by emitting the variable outside the `if` block and using the variable reference instead:
```
// Markup: `@if (expr; as alias) { {{alias}} }
var alias = block.condition;
if (alias) {
"" + alias;
}
```
PR Close #51952
This commit is contained in:
committed by
Alex Rickabaugh
parent
7426948ff6
commit
02edb43067
@@ -85,7 +85,7 @@ export function generateTypeCheckBlock(
|
||||
const tcb = new Context(
|
||||
env, domSchemaChecker, oobRecorder, meta.id, meta.boundTarget, meta.pipes, meta.schemas,
|
||||
meta.isStandalone);
|
||||
const scope = Scope.forNodes(tcb, null, tcb.boundTarget.target.template!, /* guard */ null);
|
||||
const scope = Scope.forNodes(tcb, null, null, tcb.boundTarget.target.template!, /* guard */ null);
|
||||
const ctxRawType = env.referenceType(ref);
|
||||
if (!ts.isTypeReferenceNode(ctxRawType)) {
|
||||
throw new Error(
|
||||
@@ -379,7 +379,8 @@ class TcbTemplateBodyOp extends TcbOp {
|
||||
|
||||
// Create a new Scope for the template. This constructs the list of operations for the template
|
||||
// children, as well as tracks bindings within the template.
|
||||
const tmplScope = Scope.forNodes(this.tcb, this.scope, this.template, guard);
|
||||
const tmplScope =
|
||||
Scope.forNodes(this.tcb, this.scope, this.template, this.template.children, guard);
|
||||
|
||||
// Render the template's `Scope` into its statements.
|
||||
const statements = tmplScope.render();
|
||||
@@ -1228,14 +1229,31 @@ class TcbIfOp extends TcbOp {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const branchScope = Scope.forNodes(this.tcb, this.scope, branch, null);
|
||||
|
||||
// If the expression is null, it means that it's an `else` statement.
|
||||
return branch.expression === null ?
|
||||
ts.factory.createBlock(branchScope.render()) :
|
||||
ts.factory.createIfStatement(
|
||||
tcbExpression(branch.expression, this.tcb, branchScope),
|
||||
ts.factory.createBlock(branchScope.render()), this.generateBranch(index + 1));
|
||||
if (branch.expression === null) {
|
||||
const branchScope = Scope.forNodes(this.tcb, this.scope, null, branch.children, null);
|
||||
return ts.factory.createBlock(branchScope.render());
|
||||
}
|
||||
|
||||
let branchParentScope: Scope;
|
||||
|
||||
if (branch.expressionAlias === null) {
|
||||
branchParentScope = this.scope;
|
||||
} else {
|
||||
// If the expression is aliased, we create a scope with a variable containing the expression.
|
||||
// Further down we'll use the variable instead of the expression itself in the `if` statement.
|
||||
// This allows for the type of the alias to be narrowed.
|
||||
branchParentScope = Scope.forNodes(this.tcb, this.scope, branch, [], null);
|
||||
branchParentScope.render().forEach(stmt => this.scope.addStatement(stmt));
|
||||
}
|
||||
|
||||
const branchScope = Scope.forNodes(this.tcb, branchParentScope, null, branch.children, null);
|
||||
const expression = branch.expressionAlias === null ?
|
||||
tcbExpression(branch.expression, this.tcb, branchScope) :
|
||||
branchScope.resolve(branch.expressionAlias);
|
||||
|
||||
return ts.factory.createIfStatement(
|
||||
expression, ts.factory.createBlock(branchScope.render()), this.generateBranch(index + 1));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1260,7 +1278,7 @@ class TcbSwitchOp extends TcbOp {
|
||||
for (const current of this.block.cases) {
|
||||
// Our template switch statements don't fall through so we always have a break at the end.
|
||||
const breakStatement = ts.factory.createBreakStatement();
|
||||
const clauseScope = Scope.forNodes(this.tcb, this.scope, current.children, null);
|
||||
const clauseScope = Scope.forNodes(this.tcb, this.scope, null, current.children, null);
|
||||
|
||||
if (current.expression === null) {
|
||||
clauses.push(ts.factory.createDefaultClause([...clauseScope.render(), breakStatement]));
|
||||
@@ -1294,7 +1312,7 @@ class TcbForOfOp extends TcbOp {
|
||||
}
|
||||
|
||||
override execute(): null {
|
||||
const loopScope = Scope.forNodes(this.tcb, this.scope, this.block, null);
|
||||
const loopScope = Scope.forNodes(this.tcb, this.scope, this.block, this.block.children, null);
|
||||
const initializer = ts.factory.createVariableDeclarationList(
|
||||
[ts.factory.createVariableDeclaration(this.block.item.name)], ts.NodeFlags.Const);
|
||||
// It's common to have a for loop over a nullable value (e.g. produced by the `async` pipe).
|
||||
@@ -1431,32 +1449,31 @@ class Scope {
|
||||
* Constructs a `Scope` given either a `TmplAstTemplate` or a list of `TmplAstNode`s.
|
||||
*
|
||||
* @param tcb the overall context of TCB generation.
|
||||
* @param parent the `Scope` of the parent template (if any) or `null` if this is the root
|
||||
* @param parentScope the `Scope` of the parent template (if any) or `null` if this is the root
|
||||
* `Scope`.
|
||||
* @param blockOrNodes either a `TmplAstTemplate` representing the template for which to
|
||||
* calculate the `Scope`, or a list of nodes if no outer template object is available.
|
||||
* @param scopedNode Node that provides the scope around the child nodes (e.g. a
|
||||
* `TmplAstTemplate` node exposing variables to its children).
|
||||
* @param children Child nodes that should be appended to the TCB.
|
||||
* @param guard an expression that is applied to this scope for type narrowing purposes.
|
||||
*/
|
||||
static forNodes(
|
||||
tcb: Context, parent: Scope|null,
|
||||
blockOrNodes: TmplAstTemplate|TmplAstIfBlockBranch|TmplAstForLoopBlock|TmplAstNode[],
|
||||
guard: ts.Expression|null): Scope {
|
||||
const scope = new Scope(tcb, parent, guard);
|
||||
tcb: Context, parentScope: Scope|null,
|
||||
scopedNode: TmplAstTemplate|TmplAstIfBlockBranch|TmplAstForLoopBlock|null,
|
||||
children: TmplAstNode[], guard: ts.Expression|null): Scope {
|
||||
const scope = new Scope(tcb, parentScope, guard);
|
||||
|
||||
if (parent === null && tcb.env.config.enableTemplateTypeChecker) {
|
||||
if (parentScope === null && tcb.env.config.enableTemplateTypeChecker) {
|
||||
// Add an autocompletion point for the component context.
|
||||
scope.opQueue.push(new TcbComponentContextCompletionOp(scope));
|
||||
}
|
||||
|
||||
let children: TmplAstNode[];
|
||||
|
||||
// If given an actual `TmplAstTemplate` instance, then process any additional information it
|
||||
// has.
|
||||
if (blockOrNodes instanceof TmplAstTemplate) {
|
||||
if (scopedNode instanceof TmplAstTemplate) {
|
||||
// The template's variable declarations need to be added as `TcbVariableOp`s.
|
||||
const varMap = new Map<string, TmplAstVariable>();
|
||||
|
||||
for (const v of blockOrNodes.variables) {
|
||||
for (const v of scopedNode.variables) {
|
||||
// Validate that variables on the `TmplAstTemplate` are only declared once.
|
||||
if (!varMap.has(v.name)) {
|
||||
varMap.set(v.name, v);
|
||||
@@ -1464,34 +1481,28 @@ class Scope {
|
||||
const firstDecl = varMap.get(v.name)!;
|
||||
tcb.oobRecorder.duplicateTemplateVar(tcb.id, v, firstDecl);
|
||||
}
|
||||
this.registerVariable(scope, v, new TcbTemplateVariableOp(tcb, scope, blockOrNodes, v));
|
||||
this.registerVariable(scope, v, new TcbTemplateVariableOp(tcb, scope, scopedNode, v));
|
||||
}
|
||||
children = blockOrNodes.children;
|
||||
} else if (blockOrNodes instanceof TmplAstIfBlockBranch) {
|
||||
const {expression, expressionAlias} = blockOrNodes;
|
||||
} else if (scopedNode instanceof TmplAstIfBlockBranch) {
|
||||
const {expression, expressionAlias} = scopedNode;
|
||||
if (expression !== null && expressionAlias !== null) {
|
||||
this.registerVariable(
|
||||
scope, expressionAlias,
|
||||
new TcbBlockVariableOp(
|
||||
tcb, scope, tcbExpression(expression, tcb, scope), expressionAlias));
|
||||
}
|
||||
children = blockOrNodes.children;
|
||||
} else if (blockOrNodes instanceof TmplAstForLoopBlock) {
|
||||
} else if (scopedNode instanceof TmplAstForLoopBlock) {
|
||||
this.registerVariable(
|
||||
scope, blockOrNodes.item,
|
||||
scope, scopedNode.item,
|
||||
new TcbBlockVariableOp(
|
||||
tcb, scope, ts.factory.createIdentifier(blockOrNodes.item.name), blockOrNodes.item));
|
||||
tcb, scope, ts.factory.createIdentifier(scopedNode.item.name), scopedNode.item));
|
||||
|
||||
for (const variable of Object.values(blockOrNodes.contextVariables)) {
|
||||
for (const variable of Object.values(scopedNode.contextVariables)) {
|
||||
// Note: currently all context variables are assumed to be number types.
|
||||
const type = ts.factory.createKeywordTypeNode(ts.SyntaxKind.NumberKeyword);
|
||||
this.registerVariable(
|
||||
scope, variable, new TcbBlockImplicitVariableOp(tcb, scope, type, variable));
|
||||
}
|
||||
|
||||
children = blockOrNodes.children;
|
||||
} else {
|
||||
children = blockOrNodes;
|
||||
}
|
||||
for (const node of children) {
|
||||
scope.appendNode(node);
|
||||
|
||||
@@ -1424,8 +1424,7 @@ describe('type check blocks', () => {
|
||||
}`;
|
||||
|
||||
expect(tcb(TEMPLATE))
|
||||
.toContain(
|
||||
'if ((((this).expr)) === (1)) { var _t1 = (((this).expr)) === (1); "" + (_t1); }');
|
||||
.toContain('var _t1 = (((this).expr)) === (1); if (_t1) { "" + (_t1); } }');
|
||||
});
|
||||
|
||||
it('should generate a switch block', () => {
|
||||
|
||||
@@ -3831,6 +3831,31 @@ suppress
|
||||
]);
|
||||
});
|
||||
|
||||
it('should check narrow the type in the alias', () => {
|
||||
env.write('test.ts', `
|
||||
import {Component} from '@angular/core';
|
||||
|
||||
@Component({
|
||||
template: \`@if (value; as alias) {
|
||||
{{acceptsNumber(alias)}}
|
||||
}\`,
|
||||
standalone: true,
|
||||
})
|
||||
export class Main {
|
||||
value: 'one' | 0 = 0;
|
||||
|
||||
acceptsNumber(value: number) {
|
||||
return value;
|
||||
}
|
||||
}
|
||||
`);
|
||||
|
||||
const diags = env.driveDiagnostics();
|
||||
expect(diags.map(d => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([
|
||||
`Argument of type 'string' is not assignable to parameter of type 'number'.`,
|
||||
]);
|
||||
});
|
||||
|
||||
it('should not expose the aliased expression outside of the main block', () => {
|
||||
env.write('test.ts', `
|
||||
import {Component} from '@angular/core';
|
||||
|
||||
Reference in New Issue
Block a user