mirror of
https://github.com/angular/angular.git
synced 2026-09-14 13:54:52 +08:00
refactor(migrations): add a migration for optional chainings
This migration ensure that existing code is wrapped by the `$safeNavigationMigration` magic function when necessary to maintain the pre-exisiting behavior of exisiting optional chaining expressions.
This commit is contained in:
Matthieu Riegler
@@ -145,6 +145,10 @@ bundle_entrypoints = [
|
||||
"model-output",
|
||||
"packages/core/schematics/migrations/model-output/index.js",
|
||||
],
|
||||
[
|
||||
"safe-optional-chaining",
|
||||
"packages/core/schematics/migrations/safe-optional-chaining/index.js",
|
||||
],
|
||||
]
|
||||
|
||||
rollup.rollup(
|
||||
@@ -161,6 +165,7 @@ rollup.rollup(
|
||||
"//packages/core/schematics/migrations/http-xhr-backend",
|
||||
"//packages/core/schematics/migrations/incremental-hydration",
|
||||
"//packages/core/schematics/migrations/model-output",
|
||||
"//packages/core/schematics/migrations/safe-optional-chaining",
|
||||
"//packages/core/schematics/migrations/strict-safe-navigation-narrow",
|
||||
"//packages/core/schematics/migrations/strict-templates-default",
|
||||
"//packages/core/schematics/ng-generate/cleanup-unused-imports",
|
||||
|
||||
@@ -34,6 +34,11 @@
|
||||
"version": "22.0.0",
|
||||
"description": "Migrate broken duplicate outputs",
|
||||
"factory": "./bundles/model-output.cjs#migrate"
|
||||
},
|
||||
"safe-optional-chaining": {
|
||||
"version": "22.0.0",
|
||||
"description": "Wraps optional chaining expressions in $safeNavigationMigration().",
|
||||
"factory": "./bundles/safe-optional-chaining.cjs#migrate"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,41 @@
|
||||
load("//tools:defaults.bzl", "jasmine_test", "ts_project")
|
||||
|
||||
package(
|
||||
default_visibility = [
|
||||
"//packages/core/schematics:__pkg__",
|
||||
"//packages/core/schematics/test:__pkg__",
|
||||
],
|
||||
)
|
||||
|
||||
ts_project(
|
||||
name = "safe-optional-chaining",
|
||||
srcs = glob(
|
||||
["**/*.ts"],
|
||||
exclude = ["*.spec.ts"],
|
||||
),
|
||||
deps = [
|
||||
"//:node_modules/@angular-devkit/schematics",
|
||||
"//:node_modules/typescript",
|
||||
"//packages/compiler-cli/private",
|
||||
"//packages/core/schematics/utils",
|
||||
"//packages/core/schematics/utils/tsurge",
|
||||
"//packages/core/schematics/utils/tsurge/helpers/angular_devkit",
|
||||
],
|
||||
)
|
||||
|
||||
ts_project(
|
||||
name = "test_lib",
|
||||
testonly = True,
|
||||
srcs = glob(["*.spec.ts"]),
|
||||
deps = [
|
||||
":safe-optional-chaining",
|
||||
"//:node_modules/typescript",
|
||||
"//packages/compiler-cli",
|
||||
"//packages/core/schematics/utils/tsurge",
|
||||
],
|
||||
)
|
||||
|
||||
jasmine_test(
|
||||
name = "test",
|
||||
data = [":test_lib"],
|
||||
)
|
||||
@@ -0,0 +1,20 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright Google LLC All Rights Reserved.
|
||||
*
|
||||
* Use of this source code is governed by an MIT-style license that can be
|
||||
* found in the LICENSE file at https://angular.dev/license
|
||||
*/
|
||||
|
||||
import {Rule} from '@angular-devkit/schematics';
|
||||
import {runMigrationInDevkit} from '../../utils/tsurge/helpers/angular_devkit';
|
||||
import {SafeOptionalChainingMigration} from './migration';
|
||||
|
||||
export function migrate(): Rule {
|
||||
return async (tree) => {
|
||||
await runMigrationInDevkit({
|
||||
tree,
|
||||
getMigration: () => new SafeOptionalChainingMigration(),
|
||||
});
|
||||
};
|
||||
}
|
||||
@@ -0,0 +1,752 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright Google LLC All Rights Reserved.
|
||||
*
|
||||
* Use of this source code is governed by an MIT-style license that can be
|
||||
* found in the LICENSE file at https://angular.io/license
|
||||
*/
|
||||
|
||||
import {
|
||||
AST,
|
||||
ASTWithSource,
|
||||
Binary,
|
||||
BindingPipe,
|
||||
BindingType,
|
||||
Call,
|
||||
Chain,
|
||||
Conditional,
|
||||
KeyedRead,
|
||||
LiteralPrimitive,
|
||||
NonNullAssert,
|
||||
parseTemplate,
|
||||
PrefixNot,
|
||||
PropertyRead,
|
||||
RecursiveAstVisitor,
|
||||
SafeCall,
|
||||
SafeKeyedRead,
|
||||
SafePropertyRead,
|
||||
TmplAstBoundAttribute,
|
||||
TmplAstBoundDeferredTrigger,
|
||||
TmplAstBoundEvent,
|
||||
TmplAstBoundText,
|
||||
TmplAstDeferredTrigger,
|
||||
TmplAstElement,
|
||||
TmplAstForLoopBlock,
|
||||
TmplAstIfBlockBranch,
|
||||
TmplAstLetDeclaration,
|
||||
TmplAstRecursiveVisitor,
|
||||
TmplAstSwitchBlock,
|
||||
TmplAstSwitchBlockCase,
|
||||
TmplAstSwitchBlockCaseGroup,
|
||||
TmplAstTemplate,
|
||||
TmplAstTextAttribute,
|
||||
} from '@angular/compiler';
|
||||
import {AbsoluteFsPath} from '@angular/compiler-cli';
|
||||
import ts from 'typescript';
|
||||
import {NgComponentTemplateVisitor} from '../../utils/ng_component_template';
|
||||
import {getAngularDecorators} from '../../utils/ng_decorators';
|
||||
import {
|
||||
confirmAsSerializable,
|
||||
ProgramInfo,
|
||||
projectFile,
|
||||
ProjectFile,
|
||||
Replacement,
|
||||
Serializable,
|
||||
TextUpdate,
|
||||
TsurgeFunnelMigration,
|
||||
} from '../../utils/tsurge';
|
||||
import {getPropertyNameText} from '../../utils/typescript/property_name';
|
||||
|
||||
import('@angular/compiler');
|
||||
|
||||
export interface CompilationUnitData {
|
||||
replacements: Replacement[];
|
||||
}
|
||||
|
||||
export interface MigrationConfig {
|
||||
/**
|
||||
* Whether to migrate this component template.
|
||||
*/
|
||||
shouldMigrate?: (containingFile: ProjectFile) => boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* This migration wraps optional chaining expressions in Angular templates with a call to the
|
||||
* `$safeNavigationMigration()` magic function. This function doesn't exist at runtime, but is
|
||||
* used as a marker for the Angular compiler to transform the expression to keep the legacy
|
||||
* behavior of returning `null`.
|
||||
*
|
||||
* The migration uses a top-down "sink" approach: each expression is visited with a boolean
|
||||
* `nullSensitive` context that indicates whether the consumer of the expression's value
|
||||
* distinguishes between `null` and `undefined`. Safe navigation operators (`?.`) wrap themselves
|
||||
* when their sink is null-sensitive.
|
||||
*/
|
||||
export class SafeOptionalChainingMigration extends TsurgeFunnelMigration<
|
||||
CompilationUnitData,
|
||||
CompilationUnitData
|
||||
> {
|
||||
constructor(private readonly config: MigrationConfig = {}) {
|
||||
super();
|
||||
}
|
||||
|
||||
override async analyze(info: ProgramInfo): Promise<Serializable<CompilationUnitData>> {
|
||||
const replacements: Replacement[] = [];
|
||||
|
||||
// Template Iteration
|
||||
const templateVisitor = new NgComponentTemplateVisitor(info.program.getTypeChecker());
|
||||
for (const sourceFile of info.sourceFiles) {
|
||||
templateVisitor.visitNode(sourceFile);
|
||||
}
|
||||
|
||||
for (const template of templateVisitor.resolvedTemplates) {
|
||||
const nodes = parseTemplate(template.content, template.filePath.toString(), {
|
||||
preserveWhitespaces: true,
|
||||
preserveLineEndings: true,
|
||||
preserveSignificantWhitespace: true,
|
||||
leadingTriviaChars: [],
|
||||
}).nodes;
|
||||
const file = template.inline
|
||||
? projectFile(template.container.getSourceFile(), info)
|
||||
: projectFile(template.filePath as AbsoluteFsPath, info);
|
||||
|
||||
const exprMigrator = new ExpressionMigrator(file, template.start);
|
||||
const visitor = new TmplVisitor(exprMigrator);
|
||||
|
||||
for (const node of nodes) {
|
||||
if (node.visit) {
|
||||
node.visit(visitor);
|
||||
}
|
||||
}
|
||||
|
||||
replacements.push(...exprMigrator.replacements);
|
||||
}
|
||||
|
||||
for (const sourceFile of info.sourceFiles) {
|
||||
replacements.push(...migrateHostBindingsInSourceFile(sourceFile, info));
|
||||
}
|
||||
|
||||
return confirmAsSerializable({replacements});
|
||||
}
|
||||
|
||||
override async combine(
|
||||
unitA: CompilationUnitData,
|
||||
unitB: CompilationUnitData,
|
||||
): Promise<Serializable<CompilationUnitData>> {
|
||||
const seen = new Set<string>();
|
||||
const deduped: Replacement[] = [];
|
||||
|
||||
for (const r of [...unitA.replacements, ...unitB.replacements]) {
|
||||
const key = `${r.projectFile.rootRelativePath}:${r.update.data.position}:${r.update.data.end}:${r.update.data.toInsert}`;
|
||||
if (!seen.has(key)) {
|
||||
seen.add(key);
|
||||
deduped.push(r);
|
||||
}
|
||||
}
|
||||
|
||||
return confirmAsSerializable({replacements: deduped});
|
||||
}
|
||||
|
||||
override async globalMeta(data: CompilationUnitData): Promise<Serializable<CompilationUnitData>> {
|
||||
return confirmAsSerializable(data);
|
||||
}
|
||||
|
||||
override async stats(data: CompilationUnitData) {
|
||||
return confirmAsSerializable({});
|
||||
}
|
||||
|
||||
override async migrate(data: CompilationUnitData) {
|
||||
return {replacements: data.replacements};
|
||||
}
|
||||
}
|
||||
|
||||
function migrateHostBindingsInSourceFile(
|
||||
sourceFile: ts.SourceFile,
|
||||
info: ProgramInfo,
|
||||
): Replacement[] {
|
||||
const replacements: Replacement[] = [];
|
||||
const file = projectFile(sourceFile, info);
|
||||
const typeChecker = info.program.getTypeChecker();
|
||||
|
||||
const visitNode = (node: ts.Node) => {
|
||||
if (ts.isClassDeclaration(node)) {
|
||||
const decorators = ts.getDecorators(node) ?? [];
|
||||
const ngDecorators = getAngularDecorators(typeChecker, decorators);
|
||||
|
||||
for (const decorator of ngDecorators) {
|
||||
if (decorator.name !== 'Component' && decorator.name !== 'Directive') {
|
||||
continue;
|
||||
}
|
||||
|
||||
const metadata = decorator.node.expression.arguments[0];
|
||||
if (!metadata || !ts.isObjectLiteralExpression(metadata)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
for (const prop of metadata.properties) {
|
||||
if (!ts.isPropertyAssignment(prop)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const propName = getPropertyNameText(prop.name);
|
||||
if (propName !== 'host' || !ts.isObjectLiteralExpression(prop.initializer)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
for (const hostProp of prop.initializer.properties) {
|
||||
if (
|
||||
!ts.isPropertyAssignment(hostProp) ||
|
||||
!ts.isStringLiteralLike(hostProp.initializer)
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const hostKey = getPropertyNameText(hostProp.name);
|
||||
if (hostKey === null || (!hostKey.startsWith('[') && !hostKey.startsWith('('))) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Preserve raw text between quotes/backticks so source offsets stay aligned.
|
||||
const hostExpression = hostProp.initializer.getText().slice(1, -1);
|
||||
const fakeTemplatePrefix = `<div ${hostKey}="`;
|
||||
const fakeTemplate = `${fakeTemplatePrefix}${hostExpression}"></div>`;
|
||||
const parsedNodes = parseTemplate(fakeTemplate, sourceFile.fileName, {
|
||||
preserveWhitespaces: true,
|
||||
}).nodes;
|
||||
|
||||
const hostExpressionStart = hostProp.initializer.getStart() + 1;
|
||||
const exprMigrator = new ExpressionMigrator(
|
||||
file,
|
||||
hostExpressionStart - fakeTemplatePrefix.length,
|
||||
);
|
||||
const visitor = new HostBindingVisitor(exprMigrator);
|
||||
|
||||
for (const parsedNode of parsedNodes) {
|
||||
if (parsedNode.visit) {
|
||||
parsedNode.visit(visitor);
|
||||
}
|
||||
}
|
||||
|
||||
replacements.push(...exprMigrator.replacements);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
ts.forEachChild(node, visitNode);
|
||||
};
|
||||
|
||||
visitNode(sourceFile);
|
||||
return replacements;
|
||||
}
|
||||
|
||||
/** Returns whether the given attribute is a class, style, or attribute binding. */
|
||||
function isClassStyleOrAttrBinding(attribute: TmplAstBoundAttribute): boolean {
|
||||
return (
|
||||
attribute.type === BindingType.Class ||
|
||||
attribute.type === BindingType.Style ||
|
||||
attribute.type === BindingType.Attribute ||
|
||||
(attribute.type === BindingType.Property &&
|
||||
(attribute.name === 'class' || attribute.name === 'className' || attribute.name === 'style'))
|
||||
);
|
||||
}
|
||||
|
||||
class HostBindingVisitor extends TmplAstRecursiveVisitor {
|
||||
constructor(private hostExprMigrator: ExpressionMigrator) {
|
||||
super();
|
||||
}
|
||||
|
||||
override visitBoundAttribute(attribute: TmplAstBoundAttribute) {
|
||||
if (isClassStyleOrAttrBinding(attribute)) {
|
||||
// Class/style/attr bindings use truthiness — not null-sensitive by default.
|
||||
// Safe navs inside function calls or pipes will still be wrapped by the migrator.
|
||||
attribute.value.visit(this.hostExprMigrator, false);
|
||||
} else {
|
||||
// Regular property bindings are null-sensitive.
|
||||
attribute.value.visit(this.hostExprMigrator, true);
|
||||
}
|
||||
super.visitBoundAttribute(attribute);
|
||||
}
|
||||
|
||||
override visitBoundEvent(event: TmplAstBoundEvent) {
|
||||
if (event.handler && event.handler.visit) {
|
||||
// Event handlers are not null-sensitive; safe navs inside function calls will
|
||||
// still be wrapped because function arguments are always null-sensitive.
|
||||
event.handler.visit(this.hostExprMigrator, false);
|
||||
}
|
||||
super.visitBoundEvent(event);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Visits template expressions and inserts `$safeNavigationMigration(…)` wrappers around safe
|
||||
* navigation chains whose result is consumed by a null-sensitive sink.
|
||||
*
|
||||
* The `context` parameter at every visit call is a boolean `nullSensitive` flag that answers:
|
||||
* "does the parent care whether this expression's value is `null` vs. `undefined`?"
|
||||
*
|
||||
* Propagation rules:
|
||||
* - `||`, `&&`, `??`: children are **not** null-sensitive (both normalise null/undefined).
|
||||
* - `===` / `!==` with a nullish literal on one side: the other operand **is** null-sensitive.
|
||||
* - All other binary operators: propagate the parent's null-sensitivity.
|
||||
* - `!` (prefix not): operand is **not** null-sensitive (uses truthiness).
|
||||
* - Ternary condition: **not** null-sensitive; branches inherit the parent's null-sensitivity.
|
||||
* - Function call arguments and pipe inputs: **always** null-sensitive.
|
||||
* - Receivers of property/keyed/call reads: **not** null-sensitive by default.
|
||||
* For property/keyed/call continuations, when the receiver is a safe node and the parent sink
|
||||
* is null-sensitive, wrap the full continuation node (e.g. `foo?.bar.baz`, `foo?.save()`) rather
|
||||
* than the inner safe receiver (`foo?.bar`, `foo?.save`) so runtime behavior is preserved.
|
||||
* - `NonNullAssert` (`!`): propagates the parent's null-sensitivity (type-only assertion).
|
||||
*/
|
||||
class ExpressionMigrator extends RecursiveAstVisitor {
|
||||
replacements: Replacement[] = [];
|
||||
|
||||
constructor(
|
||||
private file: ProjectFile,
|
||||
private templateStart: number,
|
||||
) {
|
||||
super();
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Safe navigation nodes — wrap when the sink is null-sensitive
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
override visitSafePropertyRead(ast: SafePropertyRead, nullSensitive: boolean): any {
|
||||
if (nullSensitive) {
|
||||
this.addReplacement(ast);
|
||||
}
|
||||
// Receiver: not null-sensitive (further access on null/undefined throws either way).
|
||||
this.visit(ast.receiver, false);
|
||||
}
|
||||
|
||||
override visitSafeKeyedRead(ast: SafeKeyedRead, nullSensitive: boolean): any {
|
||||
if (nullSensitive) {
|
||||
this.addReplacement(ast);
|
||||
}
|
||||
this.visit(ast.receiver, false);
|
||||
this.visit(ast.key, false);
|
||||
}
|
||||
|
||||
override visitSafeCall(ast: SafeCall, nullSensitive: boolean): any {
|
||||
if (nullSensitive) {
|
||||
this.addReplacement(ast);
|
||||
}
|
||||
this.visit(ast.receiver, false);
|
||||
this.visitAll(ast.args, true);
|
||||
}
|
||||
|
||||
private hasSafeReceiver(receiver: AST): boolean {
|
||||
if (
|
||||
receiver instanceof SafePropertyRead ||
|
||||
receiver instanceof SafeKeyedRead ||
|
||||
receiver instanceof SafeCall
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
if (receiver instanceof NonNullAssert) {
|
||||
return this.hasSafeReceiver(receiver.expression);
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Non-safe access — receiver is never null-sensitive
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
override visitPropertyRead(ast: PropertyRead, nullSensitive: boolean): any {
|
||||
if (nullSensitive && this.hasSafeReceiver(ast.receiver)) {
|
||||
this.addReplacement(ast);
|
||||
}
|
||||
this.visit(ast.receiver, false);
|
||||
}
|
||||
|
||||
override visitKeyedRead(ast: KeyedRead, nullSensitive: boolean): any {
|
||||
if (nullSensitive && this.hasSafeReceiver(ast.receiver)) {
|
||||
this.addReplacement(ast);
|
||||
}
|
||||
this.visit(ast.receiver, false);
|
||||
this.visit(ast.key, false);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Function calls — arguments are always null-sensitive
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
override visitCall(ast: Call, nullSensitive: boolean): any {
|
||||
if (nullSensitive && this.hasSafeReceiver(ast.receiver)) {
|
||||
this.addReplacement(ast);
|
||||
}
|
||||
this.visit(ast.receiver, false);
|
||||
this.visitAll(ast.args, true);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Pipes — input and arguments are always null-sensitive
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
override visitPipe(ast: BindingPipe, _nullSensitive: boolean): any {
|
||||
this.visit(ast.exp, true);
|
||||
this.visitAll(ast.args, true);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Binary operators
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
override visitBinary(ast: Binary, nullSensitive: boolean): any {
|
||||
if (ast.operation === '||' || ast.operation === '&&' || ast.operation === '??') {
|
||||
// These operators normalise null and undefined (both produce the same result),
|
||||
// so the operands are not null-sensitive.
|
||||
this.visit(ast.left, false);
|
||||
this.visit(ast.right, false);
|
||||
} else if (ast.operation === '===' || ast.operation === '!==') {
|
||||
// A strict comparison with a nullish literal makes the other side null-sensitive
|
||||
// (null === null is true but undefined === null is false).
|
||||
const leftIsNullish = isNullishLiteralAST(ast.left);
|
||||
const rightIsNullish = isNullishLiteralAST(ast.right);
|
||||
this.visit(ast.left, rightIsNullish);
|
||||
this.visit(ast.right, leftIsNullish);
|
||||
} else {
|
||||
// All other binary operators (<, >, +, -, …): propagate the parent's null-sensitivity
|
||||
// because null and undefined can produce different numeric results (null → 0,
|
||||
// undefined → NaN for arithmetic/comparison).
|
||||
this.visit(ast.left, nullSensitive);
|
||||
this.visit(ast.right, nullSensitive);
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Unary / logical operators
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
override visitPrefixNot(ast: PrefixNot, _nullSensitive: boolean): any {
|
||||
// Logical negation uses truthiness — null and undefined are both falsy.
|
||||
this.visit(ast.expression, false);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Ternary
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
override visitConditional(ast: Conditional, nullSensitive: boolean): any {
|
||||
// The condition is evaluated as a boolean — not null-sensitive.
|
||||
this.visit(ast.condition, false);
|
||||
// The result of a branch is consumed by the parent, so it inherits null-sensitivity.
|
||||
this.visit(ast.trueExp, nullSensitive);
|
||||
this.visit(ast.falseExp, nullSensitive);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// NonNullAssert — a compile-time type annotation, no runtime effect
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
override visitNonNullAssert(ast: NonNullAssert, nullSensitive: boolean): any {
|
||||
this.visit(ast.expression, nullSensitive);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Chain (event handler statements) — never null-sensitive
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
override visitChain(ast: Chain, _nullSensitive: boolean): any {
|
||||
this.visitAll(ast.expressions, false);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Interpolation — coerces to string, so null and undefined produce identical
|
||||
// output; sub-expressions are therefore never null-sensitive.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
override visitInterpolation(ast: {expressions: AST[]}, _nullSensitive: boolean): any {
|
||||
this.visitAll(ast.expressions, false);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// All other nodes (LiteralArray, LiteralMap, Unary, …) use the default
|
||||
// RecursiveAstVisitor which propagates the current context to every child —
|
||||
// the correct "inherit parent null-sensitivity" fallback.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
private addReplacement(ast: AST): void {
|
||||
const startArg = ast.sourceSpan.start;
|
||||
const endArg = ast.sourceSpan.end;
|
||||
|
||||
this.replacements.push(
|
||||
new Replacement(
|
||||
this.file,
|
||||
new TextUpdate({
|
||||
position: this.templateStart + endArg,
|
||||
end: this.templateStart + endArg,
|
||||
toInsert: ')',
|
||||
}),
|
||||
),
|
||||
new Replacement(
|
||||
this.file,
|
||||
new TextUpdate({
|
||||
position: this.templateStart + startArg,
|
||||
end: this.templateStart + startArg,
|
||||
toInsert: '$safeNavigationMigration(',
|
||||
}),
|
||||
),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Helper utilities
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/** Returns true if the AST node is a literal `null` or `undefined`. */
|
||||
function isNullishLiteralAST(ast: AST): boolean {
|
||||
const innerAst = ast instanceof ASTWithSource ? ast.ast : ast;
|
||||
return (
|
||||
innerAst instanceof LiteralPrimitive &&
|
||||
(innerAst.value === null || innerAst.value === undefined)
|
||||
);
|
||||
}
|
||||
|
||||
/** Returns true if the AST node is a non-null, non-undefined primitive literal. */
|
||||
function isNonNullishLiteralAST(ast: AST): boolean {
|
||||
const innerAst = ast instanceof ASTWithSource ? ast.ast : ast;
|
||||
return (
|
||||
innerAst instanceof LiteralPrimitive && innerAst.value !== null && innerAst.value !== undefined
|
||||
);
|
||||
}
|
||||
|
||||
/** Returns true if any expression in `ast` contains a strict null/undefined check. */
|
||||
function hasNullCheckInAST(ast: AST): boolean {
|
||||
const innerAst = ast instanceof ASTWithSource ? ast.ast : ast;
|
||||
|
||||
const visitor = new NullCheckVisitor();
|
||||
innerAst.visit(visitor);
|
||||
return visitor.hasNullCheck;
|
||||
}
|
||||
|
||||
class NullCheckVisitor extends RecursiveAstVisitor {
|
||||
public hasNullCheck: boolean = false;
|
||||
override visitBinary(node: Binary, context: unknown) {
|
||||
if (node.operation === '===' || node.operation === '!==') {
|
||||
const isLeftNullish = isNullishLiteralAST(node.left);
|
||||
const isRightNullish = isNullishLiteralAST(node.right);
|
||||
if (isLeftNullish || isRightNullish) {
|
||||
this.hasNullCheck = true;
|
||||
}
|
||||
}
|
||||
super.visitBinary(node, context);
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Template visitor
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Returns true if all *ngSwitchCase bindings in the given nodes (and their children)
|
||||
* are non-null/non-undefined literal expressions — meaning the switch expression
|
||||
* doesn't need null-sensitivity migration.
|
||||
*/
|
||||
function allNgSwitchCasesAreLiterals(nodes: Array<TmplAstElement | TmplAstTemplate>): boolean {
|
||||
for (const node of nodes) {
|
||||
for (const input of node.inputs) {
|
||||
if (input.name === 'ngSwitchCase' && input.value) {
|
||||
if (!isNonNullishLiteralAST(input.value)) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (node instanceof TmplAstTemplate) {
|
||||
for (const attr of node.templateAttrs) {
|
||||
if (attr instanceof TmplAstBoundAttribute && attr.name === 'ngSwitchCase' && attr.value) {
|
||||
if (!isNonNullishLiteralAST(attr.value)) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const childHosts = node.children.filter(
|
||||
(child): child is TmplAstElement | TmplAstTemplate =>
|
||||
child instanceof TmplAstElement || child instanceof TmplAstTemplate,
|
||||
);
|
||||
|
||||
if (!allNgSwitchCasesAreLiterals(childHosts)) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
class TmplVisitor extends TmplAstRecursiveVisitor {
|
||||
private migratableSwitchCases = new WeakSet<TmplAstSwitchBlockCase>();
|
||||
/**
|
||||
* Stack tracking whether the current ngSwitch context should be migrated.
|
||||
* False when all *ngSwitchCase expressions are non-null literals.
|
||||
*/
|
||||
private ngSwitchShouldMigrateStack: boolean[] = [];
|
||||
|
||||
constructor(private exprMigrator: ExpressionMigrator) {
|
||||
super();
|
||||
}
|
||||
|
||||
private shouldMigrateCurrentNgSwitchContext(): boolean {
|
||||
return this.ngSwitchShouldMigrateStack[this.ngSwitchShouldMigrateStack.length - 1] ?? true;
|
||||
}
|
||||
|
||||
private hasNgSwitchBinding(node: TmplAstElement | TmplAstTemplate): boolean {
|
||||
return (
|
||||
node.inputs.some((attr) => attr.name === 'ngSwitch') ||
|
||||
(node instanceof TmplAstTemplate &&
|
||||
node.templateAttrs.some(
|
||||
(attr: TmplAstBoundAttribute | TmplAstTextAttribute) => attr.name === 'ngSwitch',
|
||||
))
|
||||
);
|
||||
}
|
||||
|
||||
override visitElement(element: TmplAstElement) {
|
||||
const hasNgSwitch = this.hasNgSwitchBinding(element);
|
||||
if (hasNgSwitch) {
|
||||
const childHosts = element.children.filter(
|
||||
(child): child is TmplAstElement | TmplAstTemplate =>
|
||||
child instanceof TmplAstElement || child instanceof TmplAstTemplate,
|
||||
);
|
||||
this.ngSwitchShouldMigrateStack.push(!allNgSwitchCasesAreLiterals(childHosts));
|
||||
}
|
||||
|
||||
super.visitElement(element);
|
||||
|
||||
if (hasNgSwitch) {
|
||||
this.ngSwitchShouldMigrateStack.pop();
|
||||
}
|
||||
}
|
||||
|
||||
override visitBoundAttribute(attribute: TmplAstBoundAttribute) {
|
||||
if (attribute.name === 'ngForOf') {
|
||||
// ngFor/@for item expressions are not null-sensitive by default.
|
||||
// Still visit so inner null-sensitive sinks (e.g. pipes/functions) are migrated.
|
||||
attribute.value.visit(this.exprMigrator, false);
|
||||
} else if (attribute.name === 'ngIf') {
|
||||
// ngIf evaluates as a boolean; null-sensitivity only arises when the expression
|
||||
// itself contains a strict null comparison (handled inside ExpressionMigrator).
|
||||
attribute.value.visit(this.exprMigrator, false);
|
||||
} else if (attribute.name === 'ngSwitch' || attribute.name === 'ngSwitchCase') {
|
||||
attribute.value.visit(this.exprMigrator, this.shouldMigrateCurrentNgSwitchContext());
|
||||
} else if (isClassStyleOrAttrBinding(attribute)) {
|
||||
// Class/style/attr bindings use truthiness — not null-sensitive by default.
|
||||
attribute.value.visit(this.exprMigrator, false);
|
||||
} else {
|
||||
// Regular property bindings are null-sensitive.
|
||||
attribute.value.visit(this.exprMigrator, true);
|
||||
}
|
||||
super.visitBoundAttribute(attribute);
|
||||
}
|
||||
|
||||
override visitBoundEvent(event: TmplAstBoundEvent) {
|
||||
if (event.handler && event.handler.visit) {
|
||||
// Event handlers are not null-sensitive. Safe navs inside function calls
|
||||
// (e.g. `compute(user?.save())`) are still migrated because function arguments
|
||||
// are always null-sensitive in ExpressionMigrator.
|
||||
event.handler.visit(this.exprMigrator, false);
|
||||
}
|
||||
super.visitBoundEvent(event);
|
||||
}
|
||||
|
||||
override visitBoundText(text: TmplAstBoundText) {
|
||||
// Interpolation text is not null-sensitive by default; null-sensitivity is
|
||||
// introduced by pipes, function calls, or strict null comparisons inside
|
||||
// the expression, all handled by ExpressionMigrator.
|
||||
text.value.visit(this.exprMigrator, false);
|
||||
super.visitBoundText(text);
|
||||
}
|
||||
|
||||
override visitTemplate(template: TmplAstTemplate) {
|
||||
const hasNgSwitch = this.hasNgSwitchBinding(template);
|
||||
if (hasNgSwitch) {
|
||||
const childHosts = template.children.filter(
|
||||
(child): child is TmplAstElement | TmplAstTemplate =>
|
||||
child instanceof TmplAstElement || child instanceof TmplAstTemplate,
|
||||
);
|
||||
this.ngSwitchShouldMigrateStack.push(!allNgSwitchCasesAreLiterals(childHosts));
|
||||
}
|
||||
|
||||
for (const attr of template.templateAttrs) {
|
||||
if (!(attr instanceof TmplAstBoundAttribute)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (attr.name === 'ngIf') {
|
||||
attr.value.visit(this.exprMigrator, false);
|
||||
} else if (attr.name === 'ngSwitch' || attr.name === 'ngSwitchCase') {
|
||||
attr.value.visit(this.exprMigrator, this.shouldMigrateCurrentNgSwitchContext());
|
||||
} else if (attr.name === 'ngForOf') {
|
||||
// ngFor microsyntax expressions are not null-sensitive by default.
|
||||
// Still visit so nested null-sensitive sinks are handled.
|
||||
attr.value.visit(this.exprMigrator, false);
|
||||
} else {
|
||||
attr.value.visit(this.exprMigrator, true);
|
||||
}
|
||||
}
|
||||
|
||||
super.visitTemplate(template);
|
||||
|
||||
if (hasNgSwitch) {
|
||||
this.ngSwitchShouldMigrateStack.pop();
|
||||
}
|
||||
}
|
||||
|
||||
override visitIfBlockBranch(block: TmplAstIfBlockBranch) {
|
||||
if (block.expression) {
|
||||
// @if condition: not null-sensitive by default (same logic as ngIf).
|
||||
block.expression.visit(this.exprMigrator, false);
|
||||
}
|
||||
super.visitIfBlockBranch(block);
|
||||
}
|
||||
|
||||
override visitForLoopBlock(block: TmplAstForLoopBlock) {
|
||||
block.expression.visit(this.exprMigrator, false);
|
||||
block.trackBy.visit(this.exprMigrator, false);
|
||||
super.visitForLoopBlock(block);
|
||||
}
|
||||
|
||||
override visitLetDeclaration(decl: TmplAstLetDeclaration) {
|
||||
// @let value is assigned directly — null-sensitive.
|
||||
decl.value.visit(this.exprMigrator, true);
|
||||
super.visitLetDeclaration(decl);
|
||||
}
|
||||
|
||||
override visitSwitchBlock(block: TmplAstSwitchBlock) {
|
||||
const switchCases = block.groups.flatMap((group: TmplAstSwitchBlockCaseGroup) => group.cases);
|
||||
|
||||
// Don't migrate if every case expression is a non-null/non-undefined literal
|
||||
// (e.g. strings, numbers, booleans). In that case null-vs-undefined can never
|
||||
// match a case, so wrapping the switch expression would be pointless.
|
||||
const shouldMigrate = !switchCases
|
||||
.filter((switchCase) => switchCase.expression)
|
||||
.every((switchCase) => isNonNullishLiteralAST(switchCase.expression!));
|
||||
|
||||
if (shouldMigrate) {
|
||||
block.expression.visit(this.exprMigrator, true);
|
||||
for (const switchCase of switchCases) {
|
||||
this.migratableSwitchCases.add(switchCase);
|
||||
}
|
||||
}
|
||||
|
||||
super.visitSwitchBlock(block);
|
||||
}
|
||||
|
||||
override visitSwitchBlockCase(block: TmplAstSwitchBlockCase) {
|
||||
if (this.migratableSwitchCases.has(block) && block.expression) {
|
||||
block.expression.visit(this.exprMigrator, true);
|
||||
}
|
||||
super.visitSwitchBlockCase(block);
|
||||
}
|
||||
|
||||
override visitDeferredTrigger(trigger: TmplAstDeferredTrigger) {
|
||||
if (trigger instanceof TmplAstBoundDeferredTrigger) {
|
||||
// @defer (when …): same logic as @if — not null-sensitive by default.
|
||||
trigger.value.visit(this.exprMigrator, false);
|
||||
}
|
||||
super.visitDeferredTrigger(trigger);
|
||||
}
|
||||
}
|
||||
+582
@@ -0,0 +1,582 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright Google LLC All Rights Reserved.
|
||||
*
|
||||
* Use of this source code is governed by an MIT-style license that can be
|
||||
* found in the LICENSE file at https://angular.dev/license
|
||||
*/
|
||||
|
||||
import {absoluteFrom, getFileSystem} from '@angular/compiler-cli';
|
||||
import {initMockFileSystem} from '@angular/compiler-cli/private/testing';
|
||||
import {
|
||||
applyTextUpdates,
|
||||
groupReplacementsByFile,
|
||||
ProjectRootRelativePath,
|
||||
} from '../../utils/tsurge';
|
||||
import {runTsurgeMigration} from '../../utils/tsurge/testing';
|
||||
import {SafeOptionalChainingMigration} from './migration';
|
||||
|
||||
describe('SafeOptionalChainingMigration', () => {
|
||||
beforeEach(() => {
|
||||
initMockFileSystem('Native');
|
||||
});
|
||||
|
||||
it('should migrate optional chaining expressions in interpolations when applicable', async () => {
|
||||
const content = await migrateInlineTemplate(`
|
||||
{{ compute(foo?.bar) }}
|
||||
{{ compute(foo.bar.baz?.alpha) }}
|
||||
{{ compute(foo.bar?.[0]) }}
|
||||
{{ compute(foo.bar?.['abc']) }}
|
||||
{{ foo?.bar | json }}
|
||||
{{ foo.bar.baz?.alpha | json }}
|
||||
{{ foo?.bar === null ? 'A' : 'B'}}
|
||||
{{ foo?.bar === undefined ? 'A' : 'B'}}
|
||||
`);
|
||||
|
||||
expect(content).toContain('{{ compute($safeNavigationMigration(foo?.bar)) }}');
|
||||
expect(content).toContain('{{ compute($safeNavigationMigration(foo.bar.baz?.alpha)) }}');
|
||||
expect(content).toContain('{{ compute($safeNavigationMigration(foo.bar?.[0])) }}');
|
||||
expect(content).toContain("{{ compute($safeNavigationMigration(foo.bar?.['abc'])) }}");
|
||||
expect(content).toContain('{{ $safeNavigationMigration(foo?.bar) | json }}');
|
||||
expect(content).toContain('{{ $safeNavigationMigration(foo.bar.baz?.alpha) | json }}');
|
||||
expect(content).toContain("{{ $safeNavigationMigration(foo?.bar) === null ? 'A' : 'B'}}");
|
||||
expect(content).toContain("{{ $safeNavigationMigration(foo?.bar) === undefined ? 'A' : 'B'}}");
|
||||
});
|
||||
|
||||
it('should not migrate optional chaining expressions in interpolations when unnecessary', async () => {
|
||||
const content = await migrateInlineTemplate(`
|
||||
{{ ((((((((((foo?.bar))))))))) }}
|
||||
{{ foo.bar?.baz }}
|
||||
{{ foo.bar?.[0] }}
|
||||
{{ foo.bar?.['abc'] }}
|
||||
{{ foo?.bar ?? 'ok' }}
|
||||
{{ foo?.bar && 'ok' }}
|
||||
{{ foo?.bar ?? foo?.bar }}
|
||||
{{ foo?.bar || foo?.bar }}
|
||||
{{ foo?.bar && foo?.bar }}
|
||||
{{ foo.bar?.() }}
|
||||
{{ !foo?.bar }}
|
||||
`);
|
||||
expect(content).toContain('{{ ((((((((((foo?.bar))))))))) }}');
|
||||
expect(content).toContain('{{ foo.bar?.baz }}');
|
||||
expect(content).toContain('{{ foo.bar?.[0] }}');
|
||||
expect(content).toContain("{{ foo.bar?.['abc'] }}");
|
||||
expect(content).toContain("{{ foo?.bar ?? 'ok' }}");
|
||||
expect(content).toContain("{{ foo?.bar && 'ok' }}");
|
||||
expect(content).toContain('{{ foo?.bar ?? foo?.bar }}');
|
||||
expect(content).toContain('{{ foo?.bar || foo?.bar }}');
|
||||
expect(content).toContain('{{ foo?.bar && foo?.bar }}');
|
||||
expect(content).toContain('{{ foo.bar?.() }}');
|
||||
expect(content).toContain('{{ !foo?.bar }}');
|
||||
});
|
||||
|
||||
it('should not input/attribute value with interpolation', async () => {
|
||||
const content = await migrateInlineTemplate(`
|
||||
<input [id]="val-{{foo?.bar}}" />
|
||||
<div id="user-{{user?.id}}">
|
||||
`);
|
||||
|
||||
expect(content).toContain('<input [id]="val-{{foo?.bar}}" />');
|
||||
expect(content).toContain('<div id="user-{{user?.id}}">');
|
||||
});
|
||||
|
||||
it('should only migrate @if/ngIf conditional if there is a strict null check', async () => {
|
||||
const actual = await migrateInlineTemplate(`
|
||||
<div *ngIf="foo?.bar"></div>
|
||||
<div *ngIf="foo?.bar !== null"></div>
|
||||
<div *ngIf="!foo?.bar"></div>
|
||||
|
||||
@if(foo?.bar) {}
|
||||
@if(foo?.bar !== null) {}
|
||||
`);
|
||||
expect(actual).toContain('<div *ngIf="foo?.bar"></div>'); // Not migrated
|
||||
expect(actual).toContain('<div *ngIf="$safeNavigationMigration(foo?.bar) !== null"></div>');
|
||||
expect(actual).toContain('<div *ngIf="!foo?.bar"></div>'); // Negation should not be migrated
|
||||
|
||||
expect(actual).toContain('@if(foo?.bar) {}'); // Not migrated
|
||||
expect(actual).toContain('@if($safeNavigationMigration(foo?.bar) !== null) {}');
|
||||
});
|
||||
|
||||
it('should not migrate @if/ngIf conditionals that are checking for null but not with a strict equality check', async () => {
|
||||
const actual = await migrateInlineTemplate(`
|
||||
<div *ngIf="foo?.bar == null"></div>
|
||||
<div *ngIf="foo?.bar != null"></div>
|
||||
|
||||
@if(foo?.bar == null) {}
|
||||
@if(foo?.bar != null) {}
|
||||
`);
|
||||
expect(actual).toContain('<div *ngIf="foo?.bar == null"></div>');
|
||||
expect(actual).toContain('<div *ngIf="foo?.bar != null"></div>');
|
||||
|
||||
expect(actual).toContain('@if(foo?.bar == null) {}');
|
||||
expect(actual).toContain('@if(foo?.bar != null) {}');
|
||||
});
|
||||
|
||||
it('should migrate conditionals if there is a strict check against undefined', async () => {
|
||||
const actual = await migrateInlineTemplate(`
|
||||
<div *ngIf="foo?.bar === undefined"></div>
|
||||
@if(foo?.bar !== undefined) {}
|
||||
`);
|
||||
expect(actual).toContain(
|
||||
'<div *ngIf="$safeNavigationMigration(foo?.bar) === undefined"></div>',
|
||||
);
|
||||
expect(actual).toContain('@if($safeNavigationMigration(foo?.bar) !== undefined) {}');
|
||||
});
|
||||
|
||||
it('should migrate @defer when condition if there is a strict null check', async () => {
|
||||
const actual = await migrateInlineTemplate(`
|
||||
@defer (when foo?.bar === null) {
|
||||
<div>Deferred content</div>
|
||||
}
|
||||
`);
|
||||
|
||||
expect(actual).toContain('@defer (when $safeNavigationMigration(foo?.bar) === null) {');
|
||||
});
|
||||
|
||||
it('should skip simple ngFor/@for expressions', async () => {
|
||||
const actual = await migrateInlineTemplate(`
|
||||
<div *ngFor="let item of items?.list"></div>
|
||||
@for(item of items?.list; track item) {}
|
||||
`);
|
||||
expect(actual).toContain('<div *ngFor="let item of items?.list"></div>');
|
||||
expect(actual).toContain('@for(item of items?.list; track item) {}');
|
||||
// Sanity check
|
||||
expect(actual).not.toContain('$safeNavigationMigration');
|
||||
});
|
||||
|
||||
it('should migrate ngFor expressions when needed', async () => {
|
||||
const actual = await migrateInlineTemplate(
|
||||
`<div *ngFor="let user of repository?.project | users"></div>`,
|
||||
);
|
||||
expect(actual).toContain(
|
||||
'<div *ngFor="let user of $safeNavigationMigration(repository?.project) | users">',
|
||||
);
|
||||
});
|
||||
|
||||
it('should migrate @for expressions when needed', async () => {
|
||||
const actual = await migrateInlineTemplate(`
|
||||
@for (item of repository?.project | users; track item.id) {}
|
||||
`);
|
||||
expect(actual).toContain(
|
||||
'@for (item of $safeNavigationMigration(repository?.project) | users; track item.id) {}',
|
||||
);
|
||||
});
|
||||
|
||||
it('should not migrate the @for track function', async () => {
|
||||
const actual = await migrateInlineTemplate(`
|
||||
@for (item of items; track item?.id) {}
|
||||
`);
|
||||
expect(actual).toContain('@for (item of items; track item?.id) {}');
|
||||
});
|
||||
|
||||
it('should migrate a track function that contains an expression', async () => {
|
||||
const actual = await migrateInlineTemplate(`
|
||||
@for (item of items; track compute(item?.id)) {}
|
||||
`);
|
||||
expect(actual).toContain(
|
||||
'@for (item of items; track compute($safeNavigationMigration(item?.id))) {}',
|
||||
);
|
||||
});
|
||||
|
||||
it('should migrate @let declarations', async () => {
|
||||
const actual = await migrateInlineTemplate(`@let x = foo?.bar;`);
|
||||
expect(actual).toContain(`@let x = $safeNavigationMigration(foo?.bar);`);
|
||||
});
|
||||
|
||||
it('should migrate inputs and attribute bindings with optional chaining expressions', async () => {
|
||||
const actual = await migrateInlineTemplate(`
|
||||
<div [id]="user?.id"></div>
|
||||
<my-comp [userInput]="user?.name"/>
|
||||
<my-comp [userInput]="user?.name | json"/>
|
||||
<my-comp [userInput]="user?.name || 'default'"/>
|
||||
<my-comp [userInput]="user?.name && 'ok'"/>
|
||||
<my-comp [userInput]="user?.name ?? 'default'"/>
|
||||
<my-comp [userInput]="foo.bar?.()"/>
|
||||
<my-comp [userInput]="foo?.bar!"/>
|
||||
<my-comp [userInput]="foo?.bar > 0"/>
|
||||
<my-comp [userInput]="foo?.bar >= 0"/>
|
||||
<my-comp [userInput]="foo?.bar < 0"/>
|
||||
<my-comp [userInput]="foo?.bar <= 0"/>
|
||||
|
||||
`);
|
||||
expect(actual).toContain('<div [id]="$safeNavigationMigration(user?.id)"></div>');
|
||||
expect(actual).toContain('<my-comp [userInput]="$safeNavigationMigration(user?.name)"/>');
|
||||
expect(actual).toContain(
|
||||
'<my-comp [userInput]="$safeNavigationMigration(user?.name) | json"/>',
|
||||
);
|
||||
expect(actual).toContain('<my-comp [userInput]="user?.name || \'default\'"/>');
|
||||
expect(actual).toContain('<my-comp [userInput]="user?.name && \'ok\'"/>');
|
||||
expect(actual).toContain('<my-comp [userInput]="user?.name ?? \'default\'"/>');
|
||||
expect(actual).toContain('<my-comp [userInput]="$safeNavigationMigration(foo.bar?.())"/>');
|
||||
expect(actual).toContain('<my-comp [userInput]="$safeNavigationMigration(foo?.bar)!"/>');
|
||||
|
||||
// Yes there are different semantics between null & undefined checks and greater/less than comparisons
|
||||
expect(actual).toContain('<my-comp [userInput]="$safeNavigationMigration(foo?.bar) > 0"/>');
|
||||
expect(actual).toContain('<my-comp [userInput]="$safeNavigationMigration(foo?.bar) >= 0"/>');
|
||||
expect(actual).toContain('<my-comp [userInput]="$safeNavigationMigration(foo?.bar) < 0"/>');
|
||||
expect(actual).toContain('<my-comp [userInput]="$safeNavigationMigration(foo?.bar) <= 0"/>');
|
||||
});
|
||||
|
||||
it('should not migrate binding expressions when not necessary', async () => {
|
||||
const actual = await migrateInlineTemplate(`
|
||||
<my-comp [userInput]="user?.name || 'default'"/>
|
||||
<my-comp [userInput]="user?.name && 'ok'"/>
|
||||
<my-comp [userInput]="user?.name ?? 'default'"/>
|
||||
<my-comp [userInput]="foo?.isActive ? 'a' : 'b'"/>
|
||||
<my-comp [userInput]="!foo?.bar"/>
|
||||
`);
|
||||
|
||||
expect(actual).toContain('<my-comp [userInput]="user?.name || \'default\'"/>');
|
||||
expect(actual).toContain('<my-comp [userInput]="user?.name && \'ok\'"/>');
|
||||
expect(actual).toContain('<my-comp [userInput]="user?.name ?? \'default\'"/>');
|
||||
expect(actual).toContain(`<my-comp [userInput]="foo?.isActive ? 'a' : 'b'"/>`);
|
||||
expect(actual).toContain('<my-comp [userInput]="!foo?.bar"/>');
|
||||
});
|
||||
|
||||
it('should skip interpolation with no function and no pipe', async () => {
|
||||
const actual = await migrateInlineTemplate(`
|
||||
<p>{{ foo?.bar }}</p>
|
||||
<div>{{ compute(foo?.bar) }}</div>
|
||||
<span>{{ foo?.bar | json }}</span>
|
||||
`);
|
||||
expect(actual).toContain('<p>{{ foo?.bar }}</p>'); // skipped
|
||||
expect(actual).toContain('<div>{{ compute($safeNavigationMigration(foo?.bar)) }}</div>');
|
||||
expect(actual).toContain('<span>{{ $safeNavigationMigration(foo?.bar) | json }}</span>');
|
||||
});
|
||||
|
||||
it('should migrate optional chaining expressions in pipe arguments', async () => {
|
||||
const actual = await migrateInlineTemplate(`
|
||||
<p>{{ foo | myPipe:foo?.bar }}</p>
|
||||
`);
|
||||
expect(actual).toContain('<p>{{ foo | myPipe:$safeNavigationMigration(foo?.bar) }}</p>');
|
||||
});
|
||||
|
||||
it('should skip direct optional call in event handlers but migrate wrapped handlers', async () => {
|
||||
const actual = await migrateInlineTemplate(`
|
||||
<button (click)="user?.save()"></button>
|
||||
<button (click)="computed(user?.save())"></button>
|
||||
`);
|
||||
|
||||
expect(actual).toContain('<button (click)="user?.save()"></button>');
|
||||
// In `computed(user?.save())`, the optional-call continuation sits in a function
|
||||
// argument position, which is null-sensitive for consumers that distinguish
|
||||
// `null` from `undefined`; this call must be wrapped.
|
||||
expect(actual).toContain(
|
||||
'<button (click)="computed($safeNavigationMigration(user?.save()))"></button>',
|
||||
);
|
||||
});
|
||||
|
||||
it('should skip class, style, and attribute bindings that are just optional chains', async () => {
|
||||
const actual = await migrateInlineTemplate(`
|
||||
<div [class.active]="user?.active"></div>
|
||||
<div [class]="user?.classes"></div>
|
||||
<div [style.color]="user?.color"></div>
|
||||
<div [style]="user?.styles"></div>
|
||||
<div [attr.data-id]="user?.id"></div>
|
||||
|
||||
<div [class.active]="user?.active === true"></div>
|
||||
<div [class]="user?.classes || 'default'"></div>
|
||||
<div [class]="user?.active && user?.classes"></div>
|
||||
<div [class]="user?.classes ?? 'default'"></div>
|
||||
<div [attr.data-id]="user?.id || 'default'"></div>
|
||||
<div [attr.data-id]="user?.id ?? 'default'"></div>
|
||||
<div [class]="['classA', user?.classB]"></div>
|
||||
`);
|
||||
|
||||
expect(actual).toContain('<div [class.active]="user?.active"></div>');
|
||||
expect(actual).toContain('<div [class]="user?.classes"></div>');
|
||||
expect(actual).toContain('<div [style.color]="user?.color"></div>');
|
||||
expect(actual).toContain('<div [style]="user?.styles"></div>');
|
||||
expect(actual).toContain('<div [attr.data-id]="user?.id"></div>');
|
||||
expect(actual).toContain(`<div [class]="user?.classes || 'default'"></div>`);
|
||||
expect(actual).toContain(`<div [class]="user?.active && user?.classes"></div>`);
|
||||
expect(actual).toContain(`<div [class]="user?.classes ?? 'default'"></div>`);
|
||||
expect(actual).toContain(`<div [attr.data-id]="user?.id || 'default'"></div>`);
|
||||
expect(actual).toContain(`<div [attr.data-id]="user?.id ?? 'default'"></div>`);
|
||||
expect(actual).toContain(`<div [class]="['classA', user?.classB]"></div>`);
|
||||
|
||||
expect(actual).toContain('<div [class.active]="user?.active === true"></div>');
|
||||
});
|
||||
|
||||
it('should migrate some cases of class/styles/attr bindings', async () => {
|
||||
const actual = await migrateInlineTemplate(`
|
||||
<div [class.active]="checkActive(user?.id)"></div>
|
||||
<div [style.color]="getColor(user?.id)"></div>
|
||||
`);
|
||||
|
||||
expect(actual).toContain(
|
||||
'<div [class.active]="checkActive($safeNavigationMigration(user?.id))"></div>',
|
||||
);
|
||||
expect(actual).toContain(
|
||||
'<div [style.color]="getColor($safeNavigationMigration(user?.id))"></div>',
|
||||
);
|
||||
});
|
||||
|
||||
it('should migrate all expressions in ngSwitch/@switch', async () => {
|
||||
const actual = await migrateInlineTemplate(`
|
||||
<div [ngSwitch]="foo?.bar">
|
||||
<span *ngSwitchCase="foo?.bar"></span>
|
||||
</div>
|
||||
|
||||
@switch (foo?.bar) {
|
||||
@case (foo?.baz) {}
|
||||
@default {}
|
||||
}
|
||||
`);
|
||||
|
||||
expect(actual).toContain('<div [ngSwitch]="$safeNavigationMigration(foo?.bar)">');
|
||||
expect(actual).toContain('<span *ngSwitchCase="$safeNavigationMigration(foo?.bar)"></span>');
|
||||
expect(actual).toContain('@switch ($safeNavigationMigration(foo?.bar)) {');
|
||||
expect(actual).toContain('@case ($safeNavigationMigration(foo?.baz)) {}');
|
||||
});
|
||||
|
||||
it('should not migrate ngSwitch/@switch when all cases are non-null literals', async () => {
|
||||
const actual = await migrateInlineTemplate(`
|
||||
<div [ngSwitch]="foo?.bar">
|
||||
<span *ngSwitchCase="'hello'"></span>
|
||||
<span *ngSwitchCase="42"></span>
|
||||
</div>
|
||||
|
||||
@switch (foo?.bar) {
|
||||
@case ('world') {}
|
||||
@case (0) {}
|
||||
@default {}
|
||||
}
|
||||
`);
|
||||
|
||||
expect(actual).not.toContain('$safeNavigationMigration');
|
||||
});
|
||||
|
||||
it('should migrate ngSwitch/@switch if at least one case checks for null', async () => {
|
||||
const actual = await migrateInlineTemplate(`
|
||||
<div [ngSwitch]="foo?.bar">
|
||||
<span *ngSwitchCase="foo?.bar !== null"></span>
|
||||
</div>
|
||||
|
||||
@switch (foo?.bar) {
|
||||
@case (foo?.baz !== null) {}
|
||||
@default {}
|
||||
}
|
||||
|
||||
@switch (foo?.baz) {
|
||||
@case (null) {}
|
||||
@default {}
|
||||
}
|
||||
`);
|
||||
|
||||
expect(actual).toContain('<div [ngSwitch]="$safeNavigationMigration(foo?.bar)">');
|
||||
expect(actual).toContain(
|
||||
'<span *ngSwitchCase="$safeNavigationMigration(foo?.bar) !== null"></span>',
|
||||
);
|
||||
expect(actual).toContain('@switch ($safeNavigationMigration(foo?.bar)) {');
|
||||
expect(actual).toContain('@case ($safeNavigationMigration(foo?.baz) !== null) {}');
|
||||
|
||||
expect(actual).toContain('@switch ($safeNavigationMigration(foo?.baz)) {');
|
||||
});
|
||||
|
||||
it('should migrate optional-chain continuations in null-sensitive sinks', async () => {
|
||||
const content = await migrateInlineTemplate(`
|
||||
{{ computed(compute(foo?.bar.baz)?.bar.baz) }}
|
||||
`);
|
||||
// `foo?.bar.baz` and `compute(... )?.bar.baz` are optional-chain continuations.
|
||||
// In null-sensitive sinks (function args), preserving legacy null-vs-undefined behavior
|
||||
// requires wrapping these chains.
|
||||
expect(content).toContain(
|
||||
'computed($safeNavigationMigration(compute($safeNavigationMigration(foo?.bar.baz))?.bar.baz))',
|
||||
);
|
||||
});
|
||||
|
||||
it('should migrate host bindings when applicable', async () => {
|
||||
const {fs} = await runTsurgeMigration(new SafeOptionalChainingMigration(), [
|
||||
{
|
||||
name: absoluteFrom('/app.component.ts'),
|
||||
isProgramRootFile: true,
|
||||
contents: `
|
||||
import {Component} from '@angular/core';
|
||||
@Component({
|
||||
selector: 'app-root',
|
||||
template:'',
|
||||
host: {
|
||||
'[attr.data-id2]': 'computed(user?.id)',
|
||||
'[attr.data-id2]': 'user?.id | json',
|
||||
|
||||
'[class.active]': 'user?.active === null',
|
||||
'[id]': 'user?.id',
|
||||
}
|
||||
})
|
||||
export class AppComponent { foo: any; compute(a: any) {} }
|
||||
`,
|
||||
},
|
||||
]);
|
||||
const content = fs.readFile(absoluteFrom('/app.component.ts'));
|
||||
|
||||
expect(content).toContain(`'[attr.data-id2]': 'computed($safeNavigationMigration(user?.id))',`);
|
||||
expect(content).toContain(`'[attr.data-id2]': '$safeNavigationMigration(user?.id) | json',`);
|
||||
expect(content).toContain(
|
||||
`'[class.active]': '$safeNavigationMigration(user?.active) === null',`,
|
||||
);
|
||||
expect(content).toContain(`'[id]': '$safeNavigationMigration(user?.id)',`);
|
||||
});
|
||||
|
||||
it('should not migrate host bindings when not necessary', async () => {
|
||||
const {fs} = await runTsurgeMigration(new SafeOptionalChainingMigration(), [
|
||||
{
|
||||
name: absoluteFrom('/app.component.ts'),
|
||||
isProgramRootFile: true,
|
||||
contents: `
|
||||
import {Component} from '@angular/core';
|
||||
@Component({
|
||||
selector: 'app-root',
|
||||
template:'',
|
||||
host: {
|
||||
'[attr.data-id]': 'user?.id',
|
||||
'[attr.data-id]': 'user?.id ?? foo',
|
||||
'[attr.data-id]': 'user?.id || foo',
|
||||
'[attr.data-id]': 'user?.id && foo',
|
||||
'(click)': 'user?.save()'
|
||||
}
|
||||
})
|
||||
export class AppComponent { foo: any; compute(a: any) {} }
|
||||
`,
|
||||
},
|
||||
]);
|
||||
const content = fs.readFile(absoluteFrom('/app.component.ts'));
|
||||
|
||||
expect(content).toContain(`'[attr.data-id]': 'user?.id',`);
|
||||
expect(content).toContain(`'[attr.data-id]': 'user?.id ?? foo',`);
|
||||
expect(content).toContain(`'[attr.data-id]': 'user?.id || foo',`);
|
||||
expect(content).toContain(`'[attr.data-id]': 'user?.id && foo',`);
|
||||
expect(content).toContain(`'(click)': 'user?.save()'`);
|
||||
});
|
||||
|
||||
it('should handle a file that is present in multiple projects', async () => {
|
||||
const mockFs = getFileSystem();
|
||||
|
||||
const sharedFile = absoluteFrom('/app.component.ts');
|
||||
const sharedContent = `
|
||||
import {Component} from '@angular/core';
|
||||
@Component({
|
||||
selector: 'app-root',
|
||||
template: \`<div [id]="user?.id"></div>\`
|
||||
})
|
||||
export class AppComponent { user: any; }
|
||||
`;
|
||||
|
||||
mockFs.ensureDir(absoluteFrom('/'));
|
||||
mockFs.writeFile(sharedFile, sharedContent);
|
||||
|
||||
const tsconfig1 = absoluteFrom('/tsconfig.app.json');
|
||||
const tsconfig2 = absoluteFrom('/tsconfig.spec.json');
|
||||
|
||||
mockFs.writeFile(
|
||||
tsconfig1,
|
||||
JSON.stringify({compilerOptions: {strict: true, rootDir: '/'}, files: [sharedFile]}),
|
||||
);
|
||||
mockFs.writeFile(
|
||||
tsconfig2,
|
||||
JSON.stringify({compilerOptions: {strict: true, rootDir: '/'}, files: [sharedFile]}),
|
||||
);
|
||||
|
||||
const migration = new SafeOptionalChainingMigration();
|
||||
|
||||
const info1 = migration.createProgram(tsconfig1, mockFs);
|
||||
const info2 = migration.createProgram(tsconfig2, mockFs);
|
||||
|
||||
const unitData1 = await migration.analyze(info1);
|
||||
const unitData2 = await migration.analyze(info2);
|
||||
|
||||
const combined = await migration.combine(unitData1, unitData2);
|
||||
const globalMeta = await migration.globalMeta(combined);
|
||||
const {replacements} = await migration.migrate(globalMeta);
|
||||
|
||||
const updates = groupReplacementsByFile(replacements);
|
||||
const relPath = sharedFile.substring(1) as string as ProjectRootRelativePath; // strip leading '/'
|
||||
const changes = updates.get(relPath) ?? [];
|
||||
const result = applyTextUpdates(sharedContent, changes);
|
||||
|
||||
// The expression should be wrapped exactly once, not twice.
|
||||
expect(result).toContain('$safeNavigationMigration(user?.id)');
|
||||
expect(result).not.toContain('$safeNavigationMigration($safeNavigationMigration');
|
||||
});
|
||||
|
||||
it('should migrate an external template', async () => {
|
||||
const content = await migrateExternalTemplate(`
|
||||
{{ compute(foo?.bar) }}
|
||||
<div *ngIf="foo?.bar !== null"></div>
|
||||
`);
|
||||
|
||||
expect(content).toContain('{{ compute($safeNavigationMigration(foo?.bar)) }}');
|
||||
expect(content).toContain('<div *ngIf="$safeNavigationMigration(foo?.bar) !== null"></div>');
|
||||
});
|
||||
|
||||
it('should be idempotent — running twice does not double-wrap expressions', async () => {
|
||||
const input = `
|
||||
{{ compute(foo?.bar) }}
|
||||
{{ foo?.bar | json }}
|
||||
<div [id]="user?.id"></div>
|
||||
<div *ngIf="foo?.bar === null"></div>
|
||||
`;
|
||||
|
||||
// First pass: migrate fresh code
|
||||
const firstPass = await migrateInlineTemplate(input);
|
||||
|
||||
// Verify all expressions are wrapped correctly on first pass
|
||||
expect(firstPass).toContain('{{ compute($safeNavigationMigration(foo?.bar)) }}');
|
||||
expect(firstPass).toContain('{{ $safeNavigationMigration(foo?.bar) | json }}');
|
||||
expect(firstPass).toContain('<div [id]="$safeNavigationMigration(user?.id)"></div>');
|
||||
expect(firstPass).toContain('<div *ngIf="$safeNavigationMigration(foo?.bar) === null"></div>');
|
||||
|
||||
// Second pass: run migration again on already-migrated code
|
||||
const secondPass = await migrateInlineTemplate(firstPass);
|
||||
|
||||
// Verify no double-wrapping occurred
|
||||
expect(secondPass).not.toContain('$safeNavigationMigration($safeNavigationMigration');
|
||||
// The already-wrapped expressions should remain unchanged
|
||||
expect(secondPass).toContain('{{ compute($safeNavigationMigration(foo?.bar)) }}');
|
||||
expect(secondPass).toContain('{{ $safeNavigationMigration(foo?.bar) | json }}');
|
||||
expect(secondPass).toContain('<div [id]="$safeNavigationMigration(user?.id)"></div>');
|
||||
expect(secondPass).toContain('<div *ngIf="$safeNavigationMigration(foo?.bar) === null"></div>');
|
||||
});
|
||||
});
|
||||
|
||||
async function migrateInlineTemplate(template: string): Promise<string> {
|
||||
const {fs} = await runTsurgeMigration(new SafeOptionalChainingMigration(), [
|
||||
{
|
||||
name: absoluteFrom('/app.component.ts'),
|
||||
isProgramRootFile: true,
|
||||
contents: `
|
||||
import {Component} from '@angular/core';
|
||||
@Component({
|
||||
selector: 'app-root',
|
||||
template: \`
|
||||
${template}
|
||||
\`
|
||||
})
|
||||
export class AppComponent { foo: any; compute(a: any) {} }
|
||||
`,
|
||||
},
|
||||
]);
|
||||
return fs.readFile(absoluteFrom('/app.component.ts'));
|
||||
}
|
||||
|
||||
async function migrateExternalTemplate(template: string): Promise<string> {
|
||||
const {fs} = await runTsurgeMigration(new SafeOptionalChainingMigration(), [
|
||||
{
|
||||
name: absoluteFrom('/app.component.html'),
|
||||
contents: template,
|
||||
},
|
||||
{
|
||||
name: absoluteFrom('/app.component.ts'),
|
||||
isProgramRootFile: true,
|
||||
contents: `
|
||||
import {Component} from '@angular/core';
|
||||
@Component({
|
||||
selector: 'app-root',
|
||||
templateUrl: './app.component.html'
|
||||
})
|
||||
export class AppComponent { foo: any; compute(a: any) {} }
|
||||
`,
|
||||
},
|
||||
]);
|
||||
return fs.readFile(absoluteFrom('/app.component.html'));
|
||||
}
|
||||
Reference in New Issue
Block a user