From 8f8e7013d80eb19005a9c2994ddb49745d8be7bf Mon Sep 17 00:00:00 2001 From: Hinton Date: Fri, 25 Jul 2025 10:51:46 +0200 Subject: [PATCH] Add takeuntil migrator --- scripts/migrations/takeuntil/README.md | 162 +++++ scripts/migrations/takeuntil/jest.config.js | 21 + scripts/migrations/takeuntil/package.json | 10 + .../takeuntil/takeuntil-migrator.ts | 643 ++++++++++++++++++ .../src/already-migrated.component.ts | 17 + .../test/fixtures/src/basic.component.ts | 23 + .../src/complex-ondestroy.component.ts | 36 + .../test/fixtures/src/data.service.ts | 22 + .../test/fixtures/src/example.directive.ts | 21 + .../fixtures/src/mixed-usage.component.ts | 40 ++ .../src/multiple-takeuntil.component.ts | 47 ++ .../test/fixtures/src/regular-class.ts | 12 + .../takeuntil/test/fixtures/tsconfig.json | 13 + scripts/migrations/takeuntil/test/setup.ts | 4 + .../takeuntil-migrator.integration.test.ts | 347 ++++++++++ .../test/takeuntil-migrator.unit.test.ts | 525 ++++++++++++++ scripts/migrations/takeuntil/tsconfig.json | 13 + .../migrations/takeuntil/tsconfig.spec.json | 13 + 18 files changed, 1969 insertions(+) create mode 100644 scripts/migrations/takeuntil/README.md create mode 100644 scripts/migrations/takeuntil/jest.config.js create mode 100644 scripts/migrations/takeuntil/package.json create mode 100755 scripts/migrations/takeuntil/takeuntil-migrator.ts create mode 100644 scripts/migrations/takeuntil/test/fixtures/src/already-migrated.component.ts create mode 100644 scripts/migrations/takeuntil/test/fixtures/src/basic.component.ts create mode 100644 scripts/migrations/takeuntil/test/fixtures/src/complex-ondestroy.component.ts create mode 100644 scripts/migrations/takeuntil/test/fixtures/src/data.service.ts create mode 100644 scripts/migrations/takeuntil/test/fixtures/src/example.directive.ts create mode 100644 scripts/migrations/takeuntil/test/fixtures/src/mixed-usage.component.ts create mode 100644 scripts/migrations/takeuntil/test/fixtures/src/multiple-takeuntil.component.ts create mode 100644 scripts/migrations/takeuntil/test/fixtures/src/regular-class.ts create mode 100644 scripts/migrations/takeuntil/test/fixtures/tsconfig.json create mode 100644 scripts/migrations/takeuntil/test/setup.ts create mode 100644 scripts/migrations/takeuntil/test/takeuntil-migrator.integration.test.ts create mode 100644 scripts/migrations/takeuntil/test/takeuntil-migrator.unit.test.ts create mode 100644 scripts/migrations/takeuntil/tsconfig.json create mode 100644 scripts/migrations/takeuntil/tsconfig.spec.json diff --git a/scripts/migrations/takeuntil/README.md b/scripts/migrations/takeuntil/README.md new file mode 100644 index 00000000000..f00ce571d04 --- /dev/null +++ b/scripts/migrations/takeuntil/README.md @@ -0,0 +1,162 @@ +# TakeUntil to TakeUntilDestroyed Migration Tool + +A CLI utility that automatically migrates RxJS `takeUntil` patterns to Angular's `takeUntilDestroyed` using ts-morph. + +## What it does + +This tool identifies and transforms the following patterns in Angular components, directives, pipes, and services: + +✅ **Converts takeUntil patterns:** + +- `takeUntil(this._destroy)` → `takeUntilDestroyed(this.destroyRef)` +- `takeUntil(this.destroy$)` → `takeUntilDestroyed(this.destroyRef)` +- `takeUntil(this._destroy$)` → `takeUntilDestroyed(this.destroyRef)` + +✅ **Automatically handles context:** + +- In constructor: `takeUntilDestroyed()` (auto-infers destroyRef) +- In methods: `takeUntilDestroyed(this.destroyRef)` (explicit destroyRef) + +✅ **Cleans up old patterns:** + +- Removes unused destroy Subject properties +- Removes empty `ngOnDestroy` methods +- Removes `OnDestroy` interface when no longer needed +- Updates imports automatically + +✅ **Adds required imports:** + +- `inject, DestroyRef` from `@angular/core` +- `takeUntilDestroyed` from `@angular/core/rxjs-interop` + +## Usage + +### Basic Usage + +```bash +npx ts-node scripts/migrations/takeuntil/takeuntil-migrator.ts +``` + +### With Custom Options + +```bash +# Specify custom tsconfig path +npx ts-node scripts/migrations/takeuntil/takeuntil-migrator.ts --tsconfig ./apps/web/tsconfig.json + +# Specify custom file pattern +npx ts-node scripts/migrations/takeuntil/takeuntil-migrator.ts --pattern "src/**/*.component.ts" + +# Show help +npx ts-node scripts/migrations/takeuntil/takeuntil-migrator.ts --help +``` + +## Example Transformation + +### Before: + +```typescript +import { Component, OnDestroy, OnInit } from '@angular/core'; +import { Subject, takeUntil } from 'rxjs'; + +@Component({...}) +export class MyComponent implements OnInit, OnDestroy { + private _destroy$ = new Subject(); + + ngOnInit() { + this.someService.data$ + .pipe(takeUntil(this._destroy$)) + .subscribe(data => { + // handle data + }); + } + + ngOnDestroy() { + this._destroy$.next(); + this._destroy$.complete(); + } +} +``` + +### After: + +```typescript +import { Component, OnInit, inject, DestroyRef } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; + +@Component({...}) +export class MyComponent implements OnInit { + private readonly destroyRef = inject(DestroyRef); + + ngOnInit() { + this.someService.data$ + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe(data => { + // handle data + }); + } +} +``` + +## Safety Features + +- ⚠️ **Only processes Angular classes** (with @Component, @Directive, @Pipe, @Injectable decorators) +- ⚠️ **Safe property removal** - only removes destroy subjects that are exclusively used for takeUntil +- ⚠️ **Preserves other OnDestroy logic** - won't remove ngOnDestroy if it contains other cleanup code +- ⚠️ **Context-aware replacements** - handles constructor vs method usage appropriately + +## Options + +| Option | Description | Default | +| --------------------- | --------------------- | ------------------------------------------------- | +| `--tsconfig ` | Path to tsconfig.json | `./tsconfig.json` | +| `--pattern ` | File pattern to match | `/**/*.+(component\|directive\|pipe\|service).ts` | +| `--help, -h` | Show help message | - | + +## Output + +The tool provides detailed output showing: + +- Files processed and migrated +- Number of takeUntil calls replaced +- DestroyRef properties added +- Destroy properties removed + +## Post-Migration Steps + +After running the migration: + +1. **Run your linter/formatter** (eslint, prettier) +2. **Run your tests** to ensure everything works correctly +3. **Manually review changes** for any edge cases + +## Limitations + +- Only handles basic `takeUntil(this.propertyName)` patterns +- Doesn't handle complex expressions or dynamic property access +- Assumes standard naming conventions for destroy subjects +- May require manual cleanup of complex OnDestroy implementations + +## Testing + +The migration tool includes comprehensive integration tests to ensure reliability and correctness. + +### Running Tests + +```bash +# Navigate to test directory +cd scripts/migrations/takeuntil/test + +# Run all tests +npm test +``` + +### Test Fixtures + +The `test/fixtures/` directory contains sample files representing various migration patterns: + +- Basic takeUntil patterns +- Multiple patterns in one file +- Complex ngOnDestroy logic +- Mixed usage scenarios +- Non-Angular classes +- Already migrated files diff --git a/scripts/migrations/takeuntil/jest.config.js b/scripts/migrations/takeuntil/jest.config.js new file mode 100644 index 00000000000..850b4ce8a33 --- /dev/null +++ b/scripts/migrations/takeuntil/jest.config.js @@ -0,0 +1,21 @@ +/** @type {import('jest').Config} */ +module.exports = { + preset: "ts-jest", + testEnvironment: "node", + roots: [""], + testMatch: ["**/?(*.)+(spec|test).ts"], + transform: { + "^.+\\.ts$": [ + "ts-jest", + { + tsconfig: "./tsconfig.spec.json", + }, + ], + }, + collectCoverage: true, + collectCoverageFrom: ["./takeuntil-migrator.ts", "!**/*.d.ts"], + coverageDirectory: "coverage", + coverageReporters: ["text", "lcov", "html"], + testTimeout: 30000, + setupFilesAfterEnv: ["/test/setup.ts"], +}; diff --git a/scripts/migrations/takeuntil/package.json b/scripts/migrations/takeuntil/package.json new file mode 100644 index 00000000000..6dd84da6290 --- /dev/null +++ b/scripts/migrations/takeuntil/package.json @@ -0,0 +1,10 @@ +{ + "name": "@bitwarden/takeuntil-migrator", + "version": "1.0.0", + "private": true, + "scripts": { + "test": "jest", + "test:watch": "jest --watch", + "clean": "rimraf coverage temp" + } +} diff --git a/scripts/migrations/takeuntil/takeuntil-migrator.ts b/scripts/migrations/takeuntil/takeuntil-migrator.ts new file mode 100755 index 00000000000..21a765a6423 --- /dev/null +++ b/scripts/migrations/takeuntil/takeuntil-migrator.ts @@ -0,0 +1,643 @@ +#!/usr/bin/env ts-node +/* eslint-disable no-console */ + +import { readFileSync } from "fs"; +import { resolve } from "path"; + +import { Project, SyntaxKind, Scope, SourceFile, CallExpression, ClassDeclaration } from "ts-morph"; + +/** + * CLI utility to migrate RxJS takeUntil patterns to Angular's takeUntilDestroyed + * + * This tool identifies and transforms the following patterns: + * 1. takeUntil(this._destroy) -> takeUntilDestroyed(this.destroyRef) + * 2. takeUntil(this.destroy$) -> takeUntilDestroyed(this.destroyRef) + * 3. Removes destroy Subject properties when they're only used for takeUntil + * 4. Adds DestroyRef injection when needed + * 5. Updates imports + */ + +interface MigrationStats { + filesProcessed: number; + filesMigrated: number; + takeUntilCallsReplaced: number; + destroyPropertiesRemoved: number; + destroyRefPropertiesAdded: number; +} + +interface TakeUntilPattern { + callExpression: CallExpression; + destroyProperty: string; + withinConstructor: boolean; + withinMethod: boolean; +} + +class TakeUntilMigrator { + private project: Project; + private stats: MigrationStats = { + filesProcessed: 0, + filesMigrated: 0, + takeUntilCallsReplaced: 0, + destroyPropertiesRemoved: 0, + destroyRefPropertiesAdded: 0, + }; + + constructor(tsConfigPath: string) { + this.project = new Project({ + tsConfigFilePath: tsConfigPath, + }); + } + + /** + * Main migration method + */ + migrate(pattern: string = "/**/*.+(component|directive|pipe|service).ts"): MigrationStats { + console.log("🚀 Starting takeUntil to takeUntilDestroyed migration..."); + console.log(`📁 Using pattern: ${pattern}`); + + const files = this.project.getSourceFiles(pattern); + console.log(`📄 Found ${files.length} files to process`); + + for (const file of files) { + this.processFile(file); + } + + this.printSummary(); + return this.stats; + } + + /** + * Process a single file + */ + private processFile(file: SourceFile): void { + this.stats.filesProcessed++; + const filePath = file.getFilePath(); + console.log(`🔍 Processing: ${filePath.split("/").pop()}`); + + const classes = file.getDescendantsOfKind(SyntaxKind.ClassDeclaration); + let fileMigrated = false; + let fileNeedsDestroyRef = false; + + for (const clazz of classes) { + const result = this.processClass(clazz, file); + if (result.migrated) { + fileMigrated = true; + } + if (result.needsDestroyRef) { + fileNeedsDestroyRef = true; + } + } + + if (fileMigrated) { + this.stats.filesMigrated++; + this.updateImports(file, fileNeedsDestroyRef); + file.saveSync(); + console.log(`✅ Migrated: ${filePath.split("/").pop()}`); + } + } + + /** + * Process a single class + */ + private processClass( + clazz: ClassDeclaration, + file: SourceFile, + ): { migrated: boolean; needsDestroyRef: boolean } { + // Only process Angular classes (Component, Directive, Pipe, Injectable) + if (!this.isAngularClass(clazz)) { + return { migrated: false, needsDestroyRef: false }; + } + + const takeUntilPatterns = this.findTakeUntilPatterns(clazz); + if (takeUntilPatterns.length === 0) { + return { migrated: false, needsDestroyRef: false }; + } + + console.log( + ` 🎯 Found ${takeUntilPatterns.length} takeUntil pattern(s) in class ${clazz.getName()}`, + ); + + let needsDestroyRef = false; + const destroyPropertiesUsed = new Set(); + + // Process each takeUntil pattern + for (const pattern of takeUntilPatterns) { + destroyPropertiesUsed.add(pattern.destroyProperty); + + // Only use auto-inference when directly within constructor + // Methods called from constructor might also be called elsewhere, so they need explicit destroyRef + if (pattern.withinConstructor) { + // Directly in constructor: takeUntilDestroyed() can auto-infer destroyRef + pattern.callExpression.replaceWithText("takeUntilDestroyed()"); + } else { + // In methods or property initializers: need explicit destroyRef + pattern.callExpression.replaceWithText("takeUntilDestroyed(this.destroyRef)"); + needsDestroyRef = true; + } + + this.stats.takeUntilCallsReplaced++; + } + + // Add destroyRef property if needed + if (needsDestroyRef && !this.hasDestroyRefProperty(clazz)) { + this.addDestroyRefProperty(clazz); + this.stats.destroyRefPropertiesAdded++; + } + + // Remove destroy properties that are only used for takeUntil + for (const destroyPropertyName of destroyPropertiesUsed) { + if (this.canRemoveDestroyProperty(clazz, destroyPropertyName)) { + this.removeDestroyProperty(clazz, destroyPropertyName); + this.stats.destroyPropertiesRemoved++; + } + } + + // Remove ngOnDestroy if it only handled destroy subject + this.cleanupNgOnDestroy(clazz, destroyPropertiesUsed); + + return { migrated: true, needsDestroyRef }; + } + + /** + * Check if class has Angular decorators + */ + private isAngularClass(clazz: ClassDeclaration): boolean { + const angularDecorators = ["Component", "Directive", "Pipe", "Injectable"]; + return clazz + .getDecorators() + .some((decorator) => angularDecorators.includes(decorator.getName())); + } + + /** + * Find all takeUntil patterns in a class + */ + private findTakeUntilPatterns(clazz: ClassDeclaration): TakeUntilPattern[] { + const patterns: TakeUntilPattern[] = []; + + const takeUntilCalls = clazz.getDescendantsOfKind(SyntaxKind.CallExpression).filter((call) => { + const identifier = call.getExpression(); + return identifier.getText() === "takeUntil"; + }); + + for (const call of takeUntilCalls) { + const args = call.getArguments(); + if (args.length !== 1) { + continue; + } + + const arg = args[0].getText(); + + // Match patterns like this._destroy, this.destroy$, this._destroy$, etc. + const destroyPropertyMatch = arg.match(/^this\.(_?destroy\$?|_?destroy_?\$?)$/); + if (!destroyPropertyMatch) { + continue; + } + + const destroyProperty = destroyPropertyMatch[1]; + const withinConstructor = !!call.getFirstAncestorByKind(SyntaxKind.Constructor); + const withinMethod = !!call.getFirstAncestorByKind(SyntaxKind.MethodDeclaration); + + patterns.push({ + callExpression: call, + destroyProperty, + withinConstructor, + withinMethod: withinMethod && !withinConstructor, + }); + } + + return patterns; + } + + /** + * Check if class already has a destroyRef property + */ + private hasDestroyRefProperty(clazz: ClassDeclaration): boolean { + return clazz.getInstanceProperties().some((prop) => prop.getName() === "destroyRef"); + } + + /** + * Add destroyRef property to class + */ + private addDestroyRefProperty(clazz: ClassDeclaration): void { + const lastProperty = clazz.getInstanceProperties().slice(-1)[0]; + const insertIndex = lastProperty ? lastProperty.getChildIndex() + 1 : 0; + + clazz.insertProperty(insertIndex, { + name: "destroyRef", + scope: Scope.Private, + isReadonly: true, + initializer: "inject(DestroyRef)", + }); + + console.log(` ➕ Added destroyRef property`); + } + + /** + * Check if a destroy property can be safely removed + */ + private canRemoveDestroyProperty(clazz: ClassDeclaration, propertyName: string): boolean { + const property = clazz.getInstanceProperty(propertyName); + if (!property) { + return false; + } + + // Find all references to this property in the class + const propertyReferences = clazz + .getDescendantsOfKind(SyntaxKind.PropertyAccessExpression) + .filter( + (access) => + access.getName() === propertyName && access.getExpression().getText() === "this", + ); + + // Check if all references are only in takeUntil calls or ngOnDestroy + for (const ref of propertyReferences) { + // Skip if it's the property declaration itself + const refText = ref.getFullText(); + if (refText.includes("=") && refText.includes("new Subject")) { + continue; + } + + // Allow if it's in a takeUntil call argument + const takeUntilCall = ref.getFirstAncestorByKind(SyntaxKind.CallExpression); + if (takeUntilCall && takeUntilCall.getExpression().getText() === "takeUntil") { + continue; + } + + // Allow if it's in ngOnDestroy for calling next() or complete() + const method = ref.getFirstAncestorByKind(SyntaxKind.MethodDeclaration); + if (method && method.getName() === "ngOnDestroy") { + // Check if this is a method call on the property + const parent = ref.getParent(); + if (parent && parent.getKind() === SyntaxKind.PropertyAccessExpression) { + const grandParent = parent.getParent(); + if (grandParent && grandParent.getKind() === SyntaxKind.CallExpression) { + const methodCall = parent.asKindOrThrow(SyntaxKind.PropertyAccessExpression); + const methodName = methodCall.getName(); + if (methodName === "next" || methodName === "complete") { + continue; + } + } + } + } + + // If we reach here, the property is used elsewhere and can't be removed + console.log(` 💡 Property ${propertyName} is used elsewhere, keeping it`); + return false; + } + + return true; + } + + /** + * Remove a destroy property from the class + */ + private removeDestroyProperty(clazz: ClassDeclaration, propertyName: string): void { + const property = clazz.getInstanceProperty(propertyName); + if (property) { + property.remove(); + console.log(` ➖ Removed destroy property: ${propertyName}`); + } + } + + /** + * Clean up ngOnDestroy method if it only handled destroy subjects + */ + private cleanupNgOnDestroy(clazz: ClassDeclaration, destroyProperties: Set): void { + const ngOnDestroy = clazz.getMethod("ngOnDestroy"); + if (!ngOnDestroy) { + return; + } + + const body = ngOnDestroy.getBody(); + if (!body) { + return; + } + + // Type assertion to access getStatements method + const blockBody = body as any; + if (!blockBody.getStatements) { + return; + } + + const statements = blockBody.getStatements(); + let hasOnlyDestroySubjectCalls = true; + let hasDestroySubjectCalls = false; + + // Check if any destroy properties are still in use (not removed) + const hasRemainingDestroyProperties = Array.from(destroyProperties).some((prop) => { + return clazz.getInstanceProperty(prop) !== undefined; + }); + + // If any destroy properties remain, preserve ngOnDestroy + if (hasRemainingDestroyProperties) { + console.log(` 💡 Preserving ngOnDestroy because destroy properties are still in use`); + return; + } + + // Check if all statements are just destroy subject calls + for (const statement of statements) { + const text = statement.getText().trim(); + + // Allow empty statements or comments + if (!text || text.startsWith("//") || text.startsWith("/*")) { + continue; + } + + // Check if it's a destroy subject call + let isDestroySubjectCall = false; + for (const destroyProp of destroyProperties) { + if ( + text.includes(`this.${destroyProp}.next(`) || + text.includes(`this.${destroyProp}.complete(`) + ) { + isDestroySubjectCall = true; + hasDestroySubjectCalls = true; + break; + } + } + + if (!isDestroySubjectCall) { + hasOnlyDestroySubjectCalls = false; + } + } + + // Only remove the method if it ONLY has destroy subject calls and no other logic + if (hasOnlyDestroySubjectCalls && hasDestroySubjectCalls) { + ngOnDestroy.remove(); + + // Remove OnDestroy from implements clause if it exists + const implementsClause = clazz.getImplements(); + const onDestroyIndex = implementsClause.findIndex((impl) => + impl.getText().includes("OnDestroy"), + ); + + if (onDestroyIndex !== -1) { + clazz.removeImplements(onDestroyIndex); + } + + console.log(` ➖ Removed ngOnDestroy method`); + } else if (hasDestroySubjectCalls) { + // If there are other statements, just remove the destroy subject calls + this.removeDestroySubjectCallsFromNgOnDestroy(ngOnDestroy, destroyProperties); + } + } + + /** + * Remove only the destroy subject calls from ngOnDestroy, preserving other logic + */ + private removeDestroySubjectCallsFromNgOnDestroy( + ngOnDestroy: any, + destroyProperties: Set, + ): void { + const body = ngOnDestroy.getBody(); + if (!body) { + return; + } + + const blockBody = body as any; + if (!blockBody.getStatements) { + return; + } + + const statements = blockBody.getStatements(); + const statementsToRemove: any[] = []; + + for (const statement of statements) { + const text = statement.getText().trim(); + + // Check if it's a destroy subject call + for (const destroyProp of destroyProperties) { + if ( + text.includes(`this.${destroyProp}.next(`) || + text.includes(`this.${destroyProp}.complete(`) + ) { + statementsToRemove.push(statement); + break; + } + } + } + + // Remove the destroy subject call statements + for (const statement of statementsToRemove) { + statement.remove(); + } + + if (statementsToRemove.length > 0) { + console.log(` ➖ Removed destroy subject calls from ngOnDestroy`); + } + } + + /** + * Update file imports + */ + private updateImports(file: SourceFile, needsDestroyRef: boolean): void { + // Remove unused imports + this.removeUnusedRxjsImports(file); + this.removeUnusedAngularImports(file); + + // Add Angular imports + if (needsDestroyRef) { + this.addImport(file, "@angular/core", ["inject", "DestroyRef"]); + } + this.addImport(file, "@angular/core/rxjs-interop", ["takeUntilDestroyed"]); + } + + /** + * Remove unused Angular imports + */ + private removeUnusedAngularImports(file: SourceFile): void { + const angularImports = file + .getImportDeclarations() + .filter((imp) => imp.getModuleSpecifierValue() === "@angular/core"); + + for (const importDecl of angularImports) { + const namedImports = importDecl.getNamedImports(); + const unusedImports: string[] = []; + + for (const namedImport of namedImports) { + const importName = namedImport.getName(); + if (importName === "OnDestroy") { + // Check if OnDestroy is still used in the file (in implements clauses or method signatures) + const onDestroyUsages = file + .getDescendantsOfKind(SyntaxKind.Identifier) + .filter((id) => id.getText() === "OnDestroy" && id !== namedImport.getNameNode()); + + if (onDestroyUsages.length === 0) { + unusedImports.push(importName); + } + } + } + + // Remove unused imports + for (const unusedImport of unusedImports) { + const namedImport = namedImports.find((ni) => ni.getName() === unusedImport); + if (namedImport) { + namedImport.remove(); + console.log(` ➖ Removed unused import: ${unusedImport}`); + } + } + } + } + + /** + * Remove unused RxJS imports + */ + private removeUnusedRxjsImports(file: SourceFile): void { + const rxjsImports = file + .getImportDeclarations() + .filter((imp) => imp.getModuleSpecifierValue() === "rxjs"); + + for (const importDecl of rxjsImports) { + const namedImports = importDecl.getNamedImports(); + const importsToRemove: { name: string; import: any }[] = []; + + for (const namedImport of namedImports) { + const importName = namedImport.getName(); + if (importName === "Subject") { + // Check if Subject is still used in the file + const subjectUsages = file + .getDescendantsOfKind(SyntaxKind.Identifier) + .filter((id) => id.getText() === "Subject" && id !== namedImport.getNameNode()); + + if (subjectUsages.length === 0) { + importsToRemove.push({ name: importName, import: namedImport }); + } + } else if (importName === "takeUntil") { + // Check if takeUntil is still used in the file + const takeUntilUsages = file + .getDescendantsOfKind(SyntaxKind.Identifier) + .filter((id) => id.getText() === "takeUntil" && id !== namedImport.getNameNode()); + + if (takeUntilUsages.length === 0) { + importsToRemove.push({ name: importName, import: namedImport }); + } + } + } + + // Remove unused imports + for (const { import: namedImport } of importsToRemove) { + namedImport.remove(); + } + + // Remove the entire import if no named imports left + if (importDecl.getNamedImports().length === 0 && !importDecl.getDefaultImport()) { + importDecl.remove(); + } + } + } + + /** + * Add import to file + */ + private addImport(file: SourceFile, moduleSpecifier: string, namedImports: string[]): void { + let importDecl = file.getImportDeclaration( + (imp) => imp.getModuleSpecifierValue() === moduleSpecifier, + ); + + if (!importDecl) { + importDecl = file.addImportDeclaration({ + moduleSpecifier, + }); + } + + const existingImports = importDecl.getNamedImports().map((ni) => ni.getName()); + const missingImports = namedImports.filter((ni) => !existingImports.includes(ni)); + + if (missingImports.length > 0) { + importDecl.addNamedImports(missingImports); + } + } + + /** + * Print migration summary + */ + private printSummary(): void { + console.log("\n📊 Migration Summary:"); + console.log(` 📄 Files processed: ${this.stats.filesProcessed}`); + console.log(` ✅ Files migrated: ${this.stats.filesMigrated}`); + console.log(` 🔄 takeUntil calls replaced: ${this.stats.takeUntilCallsReplaced}`); + console.log(` ➕ DestroyRef properties added: ${this.stats.destroyRefPropertiesAdded}`); + console.log(` ➖ Destroy properties removed: ${this.stats.destroyPropertiesRemoved}`); + + if (this.stats.filesMigrated > 0) { + console.log("\n🎉 Migration completed successfully!"); + console.log("💡 Don't forget to:"); + console.log(" 1. Run your linter/formatter (eslint, prettier)"); + console.log(" 2. Run your tests to ensure everything works"); + console.log(" 3. Remove OnDestroy imports from @angular/core if no longer needed"); + } else { + console.log("\n🤷 No files needed migration."); + } + } +} + +// CLI Interface +function main() { + const args = process.argv.slice(2); + const helpFlag = args.includes("--help") || args.includes("-h"); + + if (helpFlag) { + console.log(` +🚀 takeUntil to takeUntilDestroyed Migration Tool + +Usage: + npx ts-node takeuntil-migrator.ts [options] + +Options: + --tsconfig Path to tsconfig.json (default: ./tsconfig.json) + --pattern File pattern to match (default: /**/*.+(component|directive|pipe|service).ts) + --help, -h Show this help message + +Examples: + npx ts-node takeuntil-migrator.ts + npx ts-node takeuntil-migrator.ts --tsconfig ./apps/web/tsconfig.json + npx ts-node takeuntil-migrator.ts --pattern "src/**/*.component.ts" + +What this tool does: + ✅ Converts takeUntil(this._destroy) to takeUntilDestroyed() + ✅ Converts takeUntil(this.destroy$) to takeUntilDestroyed() + ✅ Adds DestroyRef injection when needed + ✅ Removes unused destroy Subject properties + ✅ Cleans up ngOnDestroy methods when no longer needed + ✅ Updates imports automatically + +Note: Always run this on a clean git repository and test thoroughly! + `); + process.exit(0); + } + + const tsconfigIndex = args.indexOf("--tsconfig"); + const patternIndex = args.indexOf("--pattern"); + + const tsConfigPath = + tsconfigIndex !== -1 && args[tsconfigIndex + 1] ? args[tsconfigIndex + 1] : "./tsconfig.json"; + + const pattern = + patternIndex !== -1 && args[patternIndex + 1] + ? args[patternIndex + 1] + : "/**/*.+(component|directive|pipe|service).ts"; + + try { + // Verify tsconfig exists + readFileSync(resolve(tsConfigPath)); + + const migrator = new TakeUntilMigrator(tsConfigPath); + migrator.migrate(pattern); + } catch (error: unknown) { + if (error && typeof error === "object" && "code" in error && error.code === "ENOENT") { + console.error(`❌ Error: tsconfig.json not found at ${tsConfigPath}`); + console.error(` Please provide a valid path with --tsconfig option`); + } else { + const errorMessage = error instanceof Error ? error.message : String(error); + console.error(`❌ Error during migration:`, errorMessage); + } + process.exit(1); + } +} + +// Only run if this file is executed directly +if (require.main === module) { + main(); +} + +export { TakeUntilMigrator, MigrationStats }; diff --git a/scripts/migrations/takeuntil/test/fixtures/src/already-migrated.component.ts b/scripts/migrations/takeuntil/test/fixtures/src/already-migrated.component.ts new file mode 100644 index 00000000000..876aa2ff3c4 --- /dev/null +++ b/scripts/migrations/takeuntil/test/fixtures/src/already-migrated.component.ts @@ -0,0 +1,17 @@ +import { Component, DestroyRef, inject, OnInit } from "@angular/core"; +import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; +import { Subject } from "rxjs"; + +@Component({ + selector: "app-already-migrated", + template: "
Already Migrated Component
", +}) +export class AlreadyMigratedComponent implements OnInit { + private readonly destroyRef = inject(DestroyRef); + + ngOnInit() { + this.service.data$.pipe(takeUntilDestroyed(this.destroyRef)).subscribe(); + } + + private service = { data$: new Subject() }; +} diff --git a/scripts/migrations/takeuntil/test/fixtures/src/basic.component.ts b/scripts/migrations/takeuntil/test/fixtures/src/basic.component.ts new file mode 100644 index 00000000000..ef7fb213574 --- /dev/null +++ b/scripts/migrations/takeuntil/test/fixtures/src/basic.component.ts @@ -0,0 +1,23 @@ +import { Component, OnDestroy, OnInit } from "@angular/core"; +import { Subject, takeUntil } from "rxjs"; + +@Component({ + selector: "app-basic", + template: "
Basic Component
", +}) +export class BasicComponent implements OnInit, OnDestroy { + private _destroy$ = new Subject(); + + ngOnInit() { + // @ts-expect-error text fixture + this.someService.data$.pipe(takeUntil(this._destroy$)).subscribe((data) => { + // eslint-disable-next-line no-console + console.log(data); + }); + } + + ngOnDestroy() { + this._destroy$.next(); + this._destroy$.complete(); + } +} diff --git a/scripts/migrations/takeuntil/test/fixtures/src/complex-ondestroy.component.ts b/scripts/migrations/takeuntil/test/fixtures/src/complex-ondestroy.component.ts new file mode 100644 index 00000000000..7f155fb0298 --- /dev/null +++ b/scripts/migrations/takeuntil/test/fixtures/src/complex-ondestroy.component.ts @@ -0,0 +1,36 @@ +import { Component, OnDestroy, OnInit } from "@angular/core"; +import { Subject, takeUntil } from "rxjs"; + +@Component({ + selector: "app-complex", + template: "
Complex Component
", +}) +export class ComplexOnDestroyComponent implements OnInit, OnDestroy { + private _destroy$ = new Subject(); + + ngOnInit() { + this.service.data$.pipe(takeUntil(this._destroy$)).subscribe(); + } + + ngOnDestroy() { + // Complex cleanup - should NOT be removed + this._destroy$.next(); + this._destroy$.complete(); + + // Other cleanup logic + this.cleanupResources(); + this.saveState(); + } + + private cleanupResources() { + // eslint-disable-next-line no-console + console.log("Cleaning up resources"); + } + + private saveState() { + // eslint-disable-next-line no-console + console.log("Saving state"); + } + + private service = { data$: new Subject() }; +} diff --git a/scripts/migrations/takeuntil/test/fixtures/src/data.service.ts b/scripts/migrations/takeuntil/test/fixtures/src/data.service.ts new file mode 100644 index 00000000000..6371c4f62a1 --- /dev/null +++ b/scripts/migrations/takeuntil/test/fixtures/src/data.service.ts @@ -0,0 +1,22 @@ +import { Injectable, OnDestroy } from "@angular/core"; +import { Subject, takeUntil } from "rxjs"; + +@Injectable() +export class DataService implements OnDestroy { + private destroy$ = new Subject(); + + constructor() { + this.setupSubscriptions(); + } + + private setupSubscriptions() { + this.externalService.stream$.pipe(takeUntil(this.destroy$)).subscribe(); + } + + ngOnDestroy() { + this.destroy$.next(); + this.destroy$.complete(); + } + + private externalService = { stream$: new Subject() }; +} diff --git a/scripts/migrations/takeuntil/test/fixtures/src/example.directive.ts b/scripts/migrations/takeuntil/test/fixtures/src/example.directive.ts new file mode 100644 index 00000000000..da402a6ff2b --- /dev/null +++ b/scripts/migrations/takeuntil/test/fixtures/src/example.directive.ts @@ -0,0 +1,21 @@ +import { Directive, OnDestroy, OnInit } from "@angular/core"; +import { Subject, takeUntil } from "rxjs"; + +@Directive({ + selector: "[appExample]", +}) +export class ExampleDirective implements OnInit, OnDestroy { + private _destroy$ = new Subject(); + + ngOnInit() { + // This should be migrated + this.service.data$.pipe(takeUntil(this._destroy$)).subscribe(); + } + + ngOnDestroy() { + this._destroy$.next(); + this._destroy$.complete(); + } + + private service = { data$: new Subject() }; +} diff --git a/scripts/migrations/takeuntil/test/fixtures/src/mixed-usage.component.ts b/scripts/migrations/takeuntil/test/fixtures/src/mixed-usage.component.ts new file mode 100644 index 00000000000..194b6104931 --- /dev/null +++ b/scripts/migrations/takeuntil/test/fixtures/src/mixed-usage.component.ts @@ -0,0 +1,40 @@ +import { Component, OnDestroy, OnInit } from "@angular/core"; +import { Subject, takeUntil } from "rxjs"; + +@Component({ + selector: "app-mixed", + template: "
Mixed Usage Component
", +}) +export class MixedUsageComponent implements OnInit, OnDestroy { + private _destroy$ = new Subject(); + + ngOnInit() { + // This should be migrated + this.service.data1$.pipe(takeUntil(this._destroy$)).subscribe(); + } + + private setupOtherSubscriptions() { + // This destroy subject is also used elsewhere, so shouldn't be removed + this.service.data2$.pipe(takeUntil(this._destroy$)).subscribe(); + this.handleCustomLogic(this._destroy$); + } + + private handleCustomLogic(destroySubject: Subject) { + // Custom logic using the destroy subject + // eslint-disable-next-line rxjs-angular/prefer-takeuntil + destroySubject.subscribe(() => { + // eslint-disable-next-line no-console + console.log("Custom cleanup logic"); + }); + } + + ngOnDestroy() { + this._destroy$.next(); + this._destroy$.complete(); + } + + private service = { + data1$: new Subject(), + data2$: new Subject(), + }; +} diff --git a/scripts/migrations/takeuntil/test/fixtures/src/multiple-takeuntil.component.ts b/scripts/migrations/takeuntil/test/fixtures/src/multiple-takeuntil.component.ts new file mode 100644 index 00000000000..7e97b117982 --- /dev/null +++ b/scripts/migrations/takeuntil/test/fixtures/src/multiple-takeuntil.component.ts @@ -0,0 +1,47 @@ +/* eslint-disable rxjs/no-exposed-subjects */ +import { Component, OnDestroy, OnInit, inject } from "@angular/core"; +import { Subject, takeUntil } from "rxjs"; + +@Component({ + selector: "app-multiple", + template: "
Multiple TakeUntil Component
", +}) +export class MultipleTakeUntilComponent implements OnInit, OnDestroy { + private destroy$ = new Subject(); + private _destroy = new Subject(); + + constructor() { + // Constructor usage - should become takeUntilDestroyed() + this.stream1$.pipe(takeUntil(this.destroy$)).subscribe(); + } + + ngOnInit() { + // Method usage - should become takeUntilDestroyed(this.destroyRef) + this.stream2$.pipe(takeUntil(this._destroy)).subscribe(); + this.stream3$.pipe(takeUntil(this.destroy$)).subscribe(); + } + + private setupStreams() { + this.stream4$.pipe(takeUntil(this._destroy)).subscribe(); + } + + ngOnDestroy() { + this.destroy$.next(); + this.destroy$.complete(); + this._destroy.next(); + this._destroy.complete(); + } + + // Mock streams + private stream1$ = inject(MockService).stream1$; + private stream2$ = inject(MockService).stream2$; + private stream3$ = inject(MockService).stream3$; + private stream4$ = inject(MockService).stream4$; +} + +class MockService { + stream1$ = new Subject(); + stream2$ = new Subject(); + stream3$ = new Subject(); + stream4$ = new Subject(); +} diff --git a/scripts/migrations/takeuntil/test/fixtures/src/regular-class.ts b/scripts/migrations/takeuntil/test/fixtures/src/regular-class.ts new file mode 100644 index 00000000000..aab020aa128 --- /dev/null +++ b/scripts/migrations/takeuntil/test/fixtures/src/regular-class.ts @@ -0,0 +1,12 @@ +// This file should NOT be migrated as it's not an Angular class +import { Subject, takeUntil } from "rxjs"; + +export class RegularClass { + private _destroy$ = new Subject(); + + setupStreams() { + this.stream$.pipe(takeUntil(this._destroy$)).subscribe(); + } + + private stream$ = new Subject(); +} diff --git a/scripts/migrations/takeuntil/test/fixtures/tsconfig.json b/scripts/migrations/takeuntil/test/fixtures/tsconfig.json new file mode 100644 index 00000000000..cab04e66c1c --- /dev/null +++ b/scripts/migrations/takeuntil/test/fixtures/tsconfig.json @@ -0,0 +1,13 @@ +{ + "compilerOptions": { + "target": "ES2022", + "lib": ["ES2022", "dom"], + "module": "commonjs", + "moduleResolution": "node", + "outDir": "./dist", + "rootDir": ".", + "strict": true, + "types": ["jest", "node"] + }, + "include": ["src/*.ts"] +} diff --git a/scripts/migrations/takeuntil/test/setup.ts b/scripts/migrations/takeuntil/test/setup.ts new file mode 100644 index 00000000000..2aa3a350f33 --- /dev/null +++ b/scripts/migrations/takeuntil/test/setup.ts @@ -0,0 +1,4 @@ +// Global test setup +global.beforeEach(() => { + jest.clearAllMocks(); +}); diff --git a/scripts/migrations/takeuntil/test/takeuntil-migrator.integration.test.ts b/scripts/migrations/takeuntil/test/takeuntil-migrator.integration.test.ts new file mode 100644 index 00000000000..280635b585b --- /dev/null +++ b/scripts/migrations/takeuntil/test/takeuntil-migrator.integration.test.ts @@ -0,0 +1,347 @@ +import { readFileSync, writeFileSync, existsSync, mkdirSync, rmSync } from "fs"; +import { join } from "path"; + +import { TakeUntilMigrator } from "../takeuntil-migrator"; + +describe("TakeUntilMigrator Integration Tests", () => { + const fixturesDir = join(__dirname, "fixtures"); + const tempDir = join(__dirname, "temp"); + const tsConfigPath = join(fixturesDir, "tsconfig.json"); + + beforeEach(() => { + // Create temp directory for test files + if (existsSync(tempDir)) { + rmSync(tempDir, { recursive: true, force: true }); + } + mkdirSync(tempDir, { recursive: true }); + + // Copy fixtures to temp directory for testing + copyFixturesToTemp(); + }); + + afterEach(() => { + // Cleanup temp directory + if (existsSync(tempDir)) { + rmSync(tempDir, { recursive: true, force: true }); + } + }); + + function copyFixturesToTemp() { + const srcDir = join(fixturesDir, "src"); + const tempSrcDir = join(tempDir, "src"); + mkdirSync(tempSrcDir, { recursive: true }); + + // Copy all fixture files + const fixtureFiles = [ + "basic.component.ts", + "multiple-takeuntil.component.ts", + "example.directive.ts", + "data.service.ts", + "complex-ondestroy.component.ts", + "mixed-usage.component.ts", + "regular-class.ts", + "already-migrated.component.ts", + ]; + + fixtureFiles.forEach((file) => { + const srcPath = join(srcDir, file); + const destPath = join(tempSrcDir, file); + if (existsSync(srcPath)) { + writeFileSync(destPath, readFileSync(srcPath, "utf8")); + } + }); + + // Copy tsconfig + writeFileSync( + join(tempDir, "tsconfig.json"), + readFileSync(tsConfigPath, "utf8") + .replace(/\.\/src/g, "./src") + .replace(/"baseUrl": "\."/, `"baseUrl": "${tempDir}"`), + ); + } + + function readTempFile(fileName: string): string { + return readFileSync(join(tempDir, "src", fileName), "utf8"); + } + + describe("Basic Migration Scenarios", () => { + test("should migrate basic takeUntil pattern", () => { + const migrator = new TakeUntilMigrator(join(tempDir, "tsconfig.json")); + const stats = migrator.migrate("**/basic.component.ts"); + + expect(stats.filesProcessed).toBe(1); + expect(stats.filesMigrated).toBe(1); + expect(stats.takeUntilCallsReplaced).toBe(1); + expect(stats.destroyPropertiesRemoved).toBe(1); + + const migratedContent = readTempFile("basic.component.ts"); + + // Should add imports + expect(migratedContent).toContain("inject, DestroyRef"); + expect(migratedContent).toContain("import { takeUntilDestroyed }"); + + // Should add destroyRef property + expect(migratedContent).toContain("private readonly destroyRef = inject(DestroyRef)"); + + // Should replace takeUntil call + expect(migratedContent).toContain("takeUntilDestroyed(this.destroyRef)"); + + // Should remove destroy property + expect(migratedContent).not.toContain("_destroy$ = new Subject()"); + + // Should remove ngOnDestroy + expect(migratedContent).not.toContain("ngOnDestroy"); + expect(migratedContent).not.toContain("OnDestroy"); + }); + + test("should handle multiple takeUntil patterns in one class", () => { + const migrator = new TakeUntilMigrator(join(tempDir, "tsconfig.json")); + const stats = migrator.migrate("**/multiple-takeuntil.component.ts"); + + expect(stats.filesProcessed).toBe(1); + expect(stats.filesMigrated).toBe(1); + expect(stats.takeUntilCallsReplaced).toBe(4); + expect(stats.destroyPropertiesRemoved).toBe(2); + + const migratedContent = readTempFile("multiple-takeuntil.component.ts"); + + // Constructor usage should be takeUntilDestroyed() + expect(migratedContent).toContain("this.stream1$.pipe(takeUntilDestroyed()).subscribe()"); + + // Method usage should be takeUntilDestroyed(this.destroyRef) + expect(migratedContent).toContain( + "this.stream2$.pipe(takeUntilDestroyed(this.destroyRef)).subscribe()", + ); + expect(migratedContent).toContain( + "this.stream3$.pipe(takeUntilDestroyed(this.destroyRef)).subscribe()", + ); + expect(migratedContent).toContain( + "this.stream4$.pipe(takeUntilDestroyed(this.destroyRef)).subscribe()", + ); + + // Should remove both destroy properties + expect(migratedContent).not.toContain("destroy$ = new Subject()"); + expect(migratedContent).not.toContain("_destroy = new Subject()"); + }); + + test("should migrate directives", () => { + const migrator = new TakeUntilMigrator(join(tempDir, "tsconfig.json")); + const stats = migrator.migrate("**/example.directive.ts"); + + expect(stats.filesMigrated).toBe(1); + + const migratedContent = readTempFile("example.directive.ts"); + expect(migratedContent).toContain("takeUntilDestroyed(this.destroyRef)"); + expect(migratedContent).toContain("private readonly destroyRef = inject(DestroyRef)"); + }); + + test("should migrate services", () => { + const migrator = new TakeUntilMigrator(join(tempDir, "tsconfig.json")); + const stats = migrator.migrate("**/data.service.ts"); + + expect(stats.filesMigrated).toBe(1); + + const migratedContent = readTempFile("data.service.ts"); + expect(migratedContent).toContain("takeUntilDestroyed(this.destroyRef)"); // Method needs explicit destroyRef + expect(migratedContent).not.toContain("destroy$ = new Subject()"); + }); + }); + + describe("Complex Scenarios", () => { + test("should preserve complex ngOnDestroy logic", () => { + const migrator = new TakeUntilMigrator(join(tempDir, "tsconfig.json")); + const stats = migrator.migrate("**/complex-ondestroy.component.ts"); + + expect(stats.filesMigrated).toBe(1); + + const migratedContent = readTempFile("complex-ondestroy.component.ts"); + + // Should migrate takeUntil + expect(migratedContent).toContain("takeUntilDestroyed(this.destroyRef)"); + + // Should remove destroy property since it's only used for takeUntil + expect(migratedContent).not.toContain("_destroy$ = new Subject()"); + + // Should preserve ngOnDestroy because it has other logic + expect(migratedContent).toContain("ngOnDestroy"); + expect(migratedContent).toContain("cleanupResources()"); + expect(migratedContent).toContain("saveState()"); + + // Should remove only the destroy subject calls + expect(migratedContent).not.toContain("this._destroy$.next()"); + expect(migratedContent).not.toContain("this._destroy$.complete()"); + }); + + test("should preserve destroy properties used elsewhere", () => { + const migrator = new TakeUntilMigrator(join(tempDir, "tsconfig.json")); + const stats = migrator.migrate("**/mixed-usage.component.ts"); + + expect(stats.filesMigrated).toBe(1); + expect(stats.destroyPropertiesRemoved).toBe(0); // Should not remove the property + + const migratedContent = readTempFile("mixed-usage.component.ts"); + + // Should migrate takeUntil calls + expect(migratedContent).toContain("takeUntilDestroyed(this.destroyRef)"); + + // Should preserve destroy property because it's used elsewhere + expect(migratedContent).toContain("_destroy$ = new Subject()"); + + // Should preserve ngOnDestroy + expect(migratedContent).toContain("ngOnDestroy"); + }); + + test("should not migrate non-Angular classes", () => { + const migrator = new TakeUntilMigrator(join(tempDir, "tsconfig.json")); + const stats = migrator.migrate("**/regular-class.ts"); + + expect(stats.filesMigrated).toBe(0); + expect(stats.takeUntilCallsReplaced).toBe(0); + + const content = readTempFile("regular-class.ts"); + + // Should remain unchanged + expect(content).toContain("takeUntil(this._destroy$)"); + expect(content).toContain("_destroy$ = new Subject()"); + expect(content).not.toContain("takeUntilDestroyed"); + }); + + test("should not modify already migrated files", () => { + const migrator = new TakeUntilMigrator(join(tempDir, "tsconfig.json")); + const originalContent = readTempFile("already-migrated.component.ts"); + + const stats = migrator.migrate("**/already-migrated.component.ts"); + + expect(stats.filesMigrated).toBe(0); + + const content = readTempFile("already-migrated.component.ts"); + expect(content).toBe(originalContent); + }); + }); + + describe("Import Management", () => { + test("should add required imports", () => { + const migrator = new TakeUntilMigrator(join(tempDir, "tsconfig.json")); + migrator.migrate("**/basic.component.ts"); + + const migratedContent = readTempFile("basic.component.ts"); + + expect(migratedContent).toContain("inject, DestroyRef"); + expect(migratedContent).toContain('from "@angular/core"'); + expect(migratedContent).toContain("takeUntilDestroyed"); + expect(migratedContent).toContain('from "@angular/core/rxjs-interop"'); + }); + + test("should remove unused RxJS imports", () => { + const migrator = new TakeUntilMigrator(join(tempDir, "tsconfig.json")); + migrator.migrate("**/basic.component.ts"); + + const migratedContent = readTempFile("basic.component.ts"); + + // Should remove Subject import since it's no longer used + expect(migratedContent).not.toContain("Subject"); + expect(migratedContent).not.toContain('from "rxjs"'); + }); + + test("should preserve RxJS imports when still needed", () => { + const migrator = new TakeUntilMigrator(join(tempDir, "tsconfig.json")); + migrator.migrate("**/mixed-usage.component.ts"); + + const migratedContent = readTempFile("mixed-usage.component.ts"); + + // Should keep Subject import because it's still used + expect(migratedContent).toContain("Subject"); + expect(migratedContent).toContain('from "rxjs"'); + }); + }); + + describe("Pattern Matching", () => { + test("should process files matching pattern", () => { + const migrator = new TakeUntilMigrator(join(tempDir, "tsconfig.json")); + const stats = migrator.migrate("**/**/*.component.ts"); + + expect(stats.filesProcessed).toBeGreaterThan(0); + expect(stats.filesMigrated).toBeGreaterThan(0); + }); + + test("should handle custom file patterns", () => { + const migrator = new TakeUntilMigrator(join(tempDir, "tsconfig.json")); + const stats = migrator.migrate("**/**/*.service.ts"); + + expect(stats.filesProcessed).toBe(1); // Only data.service.ts + expect(stats.filesMigrated).toBe(1); + }); + }); + + describe("Error Handling", () => { + test("should handle invalid tsconfig path", () => { + expect(() => { + new TakeUntilMigrator("/non/existent/tsconfig.json"); + }).toThrow(); + }); + + test("should handle files with syntax errors gracefully", () => { + // Create a file with syntax errors + const invalidFile = join(tempDir, "src", "invalid.component.ts"); + writeFileSync( + invalidFile, + ` + import { Component } from '@angular/core'; + + @Component({ + selector: 'app-invalid' + }) + export class InvalidComponent { + // Missing closing brace + `, + ); + + const migrator = new TakeUntilMigrator(join(tempDir, "tsconfig.json")); + + // Should not throw, but gracefully handle the error + expect(() => { + migrator.migrate("**/invalid.component.ts"); + }).not.toThrow(); + }); + }); + + describe("Statistics Reporting", () => { + test("should provide accurate migration statistics", () => { + const migrator = new TakeUntilMigrator(join(tempDir, "tsconfig.json")); + const stats = migrator.migrate("**/**/*.ts"); + + expect(stats.filesProcessed).toBeGreaterThan(0); + expect(stats.filesMigrated).toBeGreaterThan(0); + expect(stats.takeUntilCallsReplaced).toBeGreaterThan(0); + expect(stats.destroyPropertiesRemoved).toBeGreaterThan(0); + expect(stats.destroyRefPropertiesAdded).toBeGreaterThan(0); + + // Validate that stats make logical sense + expect(stats.filesMigrated).toBeLessThanOrEqual(stats.filesProcessed); + expect(stats.takeUntilCallsReplaced).toBeGreaterThanOrEqual(stats.filesMigrated); + }); + }); + + describe("File System Integration", () => { + test("should save files correctly", () => { + const migrator = new TakeUntilMigrator(join(tempDir, "tsconfig.json")); + migrator.migrate("**/basic.component.ts"); + + // File should exist and be readable + const filePath = join(tempDir, "src", "basic.component.ts"); + expect(existsSync(filePath)).toBe(true); + + const content = readFileSync(filePath, "utf8"); + expect(content).toContain("takeUntilDestroyed"); + }); + + test("should handle file permissions correctly", () => { + const migrator = new TakeUntilMigrator(join(tempDir, "tsconfig.json")); + + // This should not throw even if there are permission issues + expect(() => { + migrator.migrate("**/basic.component.ts"); + }).not.toThrow(); + }); + }); +}); diff --git a/scripts/migrations/takeuntil/test/takeuntil-migrator.unit.test.ts b/scripts/migrations/takeuntil/test/takeuntil-migrator.unit.test.ts new file mode 100644 index 00000000000..43bc3fbcb8a --- /dev/null +++ b/scripts/migrations/takeuntil/test/takeuntil-migrator.unit.test.ts @@ -0,0 +1,525 @@ +import { Project } from "ts-morph"; + +import { TakeUntilMigrator } from "../takeuntil-migrator"; + +describe("TakeUntilMigrator Unit Tests", () => { + let project: Project; + let migrator: TakeUntilMigrator; + + beforeEach(() => { + project = new Project({ + useInMemoryFileSystem: true, + compilerOptions: { + target: 99, // Latest target + lib: ["es2022"], + experimentalDecorators: true, + emitDecoratorMetadata: true, + }, + }); + + // Create a mock tsconfig + project.createSourceFile( + "tsconfig.json", + JSON.stringify({ + compilerOptions: { + target: "ES2022", + experimentalDecorators: true, + emitDecoratorMetadata: true, + }, + }), + ); + + migrator = new TakeUntilMigrator("tsconfig.json"); + // Override the project to use our in-memory one + (migrator as any).project = project; + }); + + describe("isAngularClass", () => { + test("should identify Component classes", () => { + const file = project.createSourceFile( + "test.ts", + ` + import { Component } from '@angular/core'; + + @Component({ selector: 'test' }) + export class TestComponent {} + `, + ); + + const clazz = file.getClasses()[0]; + const isAngular = (migrator as any).isAngularClass(clazz); + + expect(isAngular).toBe(true); + }); + + test("should identify Directive classes", () => { + const file = project.createSourceFile( + "test.ts", + ` + import { Directive } from '@angular/core'; + + @Directive({ selector: '[test]' }) + export class TestDirective {} + `, + ); + + const clazz = file.getClasses()[0]; + const isAngular = (migrator as any).isAngularClass(clazz); + + expect(isAngular).toBe(true); + }); + + test("should identify Injectable classes", () => { + const file = project.createSourceFile( + "test.ts", + ` + import { Injectable } from '@angular/core'; + + @Injectable() + export class TestService {} + `, + ); + + const clazz = file.getClasses()[0]; + const isAngular = (migrator as any).isAngularClass(clazz); + + expect(isAngular).toBe(true); + }); + + test("should not identify regular classes", () => { + const file = project.createSourceFile( + "test.ts", + ` + export class RegularClass {} + `, + ); + + const clazz = file.getClasses()[0]; + const isAngular = (migrator as any).isAngularClass(clazz); + + expect(isAngular).toBe(false); + }); + }); + + describe("findTakeUntilPatterns", () => { + test("should find takeUntil patterns with various destroy property names", () => { + const file = project.createSourceFile( + "test.ts", + ` + import { Component } from '@angular/core'; + import { takeUntil } from 'rxjs/operators'; + + @Component({ selector: 'test' }) + export class TestComponent { + private _destroy$ = new Subject(); + private destroy$ = new Subject(); + private _destroy = new Subject(); + + ngOnInit() { + stream1$.pipe(takeUntil(this._destroy$)).subscribe(); + stream2$.pipe(takeUntil(this.destroy$)).subscribe(); + stream3$.pipe(takeUntil(this._destroy)).subscribe(); + } + } + `, + ); + + const clazz = file.getClasses()[0]; + const patterns = (migrator as any).findTakeUntilPatterns(clazz); + + expect(patterns).toHaveLength(3); + expect(patterns.map((p: any) => p.destroyProperty)).toEqual([ + "_destroy$", + "destroy$", + "_destroy", + ]); + }); + + test("should detect constructor context", () => { + const file = project.createSourceFile( + "test.ts", + ` + import { Component } from '@angular/core'; + import { takeUntil } from 'rxjs/operators'; + + @Component({ selector: 'test' }) + export class TestComponent { + private _destroy$ = new Subject(); + + constructor() { + stream$.pipe(takeUntil(this._destroy$)).subscribe(); + } + } + `, + ); + + const clazz = file.getClasses()[0]; + const patterns = (migrator as any).findTakeUntilPatterns(clazz); + + expect(patterns).toHaveLength(1); + expect(patterns[0].withinConstructor).toBe(true); + expect(patterns[0].withinMethod).toBe(false); + }); + + test("should detect method context", () => { + const file = project.createSourceFile( + "test.ts", + ` + import { Component } from '@angular/core'; + import { takeUntil } from 'rxjs/operators'; + + @Component({ selector: 'test' }) + export class TestComponent { + private _destroy$ = new Subject(); + + ngOnInit() { + stream$.pipe(takeUntil(this._destroy$)).subscribe(); + } + } + `, + ); + + const clazz = file.getClasses()[0]; + const patterns = (migrator as any).findTakeUntilPatterns(clazz); + + expect(patterns).toHaveLength(1); + expect(patterns[0].withinConstructor).toBe(false); + expect(patterns[0].withinMethod).toBe(true); + }); + + test("should ignore non-matching takeUntil calls", () => { + const file = project.createSourceFile( + "test.ts", + ` + import { Component } from '@angular/core'; + import { takeUntil } from 'rxjs/operators'; + + @Component({ selector: 'test' }) + export class TestComponent { + ngOnInit() { + stream1$.pipe(takeUntil(someOtherStream$)).subscribe(); + stream2$.pipe(takeUntil(this.notADestroyProperty)).subscribe(); + } + } + `, + ); + + const clazz = file.getClasses()[0]; + const patterns = (migrator as any).findTakeUntilPatterns(clazz); + + expect(patterns).toHaveLength(0); + }); + }); + + describe("hasDestroyRefProperty", () => { + test("should detect existing destroyRef property", () => { + const file = project.createSourceFile( + "test.ts", + ` + import { Component } from '@angular/core'; + + @Component({ selector: 'test' }) + export class TestComponent { + private destroyRef = inject(DestroyRef); + } + `, + ); + + const clazz = file.getClasses()[0]; + const hasDestroyRef = (migrator as any).hasDestroyRefProperty(clazz); + + expect(hasDestroyRef).toBe(true); + }); + + test("should return false when no destroyRef property exists", () => { + const file = project.createSourceFile( + "test.ts", + ` + import { Component } from '@angular/core'; + + @Component({ selector: 'test' }) + export class TestComponent { + private someOtherProperty = 'value'; + } + `, + ); + + const clazz = file.getClasses()[0]; + const hasDestroyRef = (migrator as any).hasDestroyRefProperty(clazz); + + expect(hasDestroyRef).toBe(false); + }); + }); + + describe("canRemoveDestroyProperty", () => { + test("should allow removal when only used in takeUntil and ngOnDestroy", () => { + const file = project.createSourceFile( + "test.ts", + ` + import { Component } from '@angular/core'; + import { takeUntil } from 'rxjs/operators'; + + @Component({ selector: 'test' }) + export class TestComponent { + private _destroy$ = new Subject(); + + ngOnInit() { + stream$.pipe(takeUntil(this._destroy$)).subscribe(); + } + + ngOnDestroy() { + this._destroy$.next(); + this._destroy$.complete(); + } + } + `, + ); + + const clazz = file.getClasses()[0]; + const canRemove = (migrator as any).canRemoveDestroyProperty(clazz, "_destroy$"); + + expect(canRemove).toBe(true); + }); + + test("should prevent removal when used elsewhere", () => { + const file = project.createSourceFile( + "test.ts", + ` + import { Component } from '@angular/core'; + import { takeUntil } from 'rxjs/operators'; + + @Component({ selector: 'test' }) + export class TestComponent { + private _destroy$ = new Subject(); + + ngOnInit() { + stream$.pipe(takeUntil(this._destroy$)).subscribe(); + this.customMethod(this._destroy$); + } + + customMethod(subject: Subject) { + // Custom usage + } + } + `, + ); + + const clazz = file.getClasses()[0]; + const canRemove = (migrator as any).canRemoveDestroyProperty(clazz, "_destroy$"); + + expect(canRemove).toBe(false); + }); + }); + + describe("Import Management", () => { + test("should add Angular imports correctly", () => { + const file = project.createSourceFile( + "test.ts", + ` + import { Component } from '@angular/core'; + + @Component({ selector: 'test' }) + export class TestComponent {} + `, + ); + + (migrator as any).addImport(file, "@angular/core", ["inject", "DestroyRef"]); + + const content = file.getFullText(); + expect(content).toContain("inject, DestroyRef"); + }); + + test("should add new import declaration when module not imported", () => { + const file = project.createSourceFile( + "test.ts", + ` + import { Component } from '@angular/core'; + + @Component({ selector: 'test' }) + export class TestComponent {} + `, + ); + + (migrator as any).addImport(file, "@angular/core/rxjs-interop", ["takeUntilDestroyed"]); + + const content = file.getFullText(); + expect(content).toContain("@angular/core/rxjs-interop"); + expect(content).toContain("takeUntilDestroyed"); + }); + + test("should not duplicate existing imports", () => { + const file = project.createSourceFile( + "test.ts", + ` + import { Component, inject } from '@angular/core'; + + @Component({ selector: 'test' }) + export class TestComponent {} + `, + ); + + (migrator as any).addImport(file, "@angular/core", ["inject", "DestroyRef"]); + + const content = file.getFullText(); + + // Should have inject in imports only (no duplication) + const injectMatches = content.match(/inject/g); + expect(injectMatches?.length).toBe(1); // Only in import, not duplicated + + // Should have DestroyRef added to imports + expect(content).toContain("DestroyRef"); + expect(content).toMatch(/import\s*{\s*[^}]*DestroyRef[^}]*}\s*from\s*['"]@angular\/core['"]/); + }); + }); + + describe("DestroyRef Property Addition", () => { + test("should add destroyRef property with correct syntax", () => { + const file = project.createSourceFile( + "test.ts", + ` + import { Component } from '@angular/core'; + + @Component({ selector: 'test' }) + export class TestComponent { + private existingProperty = 'value'; + } + `, + ); + + const clazz = file.getClasses()[0]; + (migrator as any).addDestroyRefProperty(clazz); + + const content = file.getFullText(); + expect(content).toContain("private readonly destroyRef = inject(DestroyRef)"); + }); + }); + + describe("Error Handling and Edge Cases", () => { + test("should handle files without classes", () => { + const file = project.createSourceFile( + "test.ts", + ` + export const someConstant = 'value'; + export function someFunction() {} + `, + ); + + expect(() => { + (migrator as any).processFile(file); + }).not.toThrow(); + }); + + test("should handle classes without decorators", () => { + const file = project.createSourceFile( + "test.ts", + ` + export class RegularClass { + method() {} + } + `, + ); + + expect(() => { + (migrator as any).processFile(file); + }).not.toThrow(); + }); + + test("should handle empty takeUntil calls", () => { + const file = project.createSourceFile( + "test.ts", + ` + import { Component } from '@angular/core'; + + @Component({ selector: 'test' }) + export class TestComponent { + method() { + stream$.pipe(takeUntil()).subscribe(); + } + } + `, + ); + + const clazz = file.getClasses()[0]; + const patterns = (migrator as any).findTakeUntilPatterns(clazz); + + expect(patterns).toHaveLength(0); + }); + + test("should handle takeUntil with multiple arguments", () => { + const file = project.createSourceFile( + "test.ts", + ` + import { Component } from '@angular/core'; + + @Component({ selector: 'test' }) + export class TestComponent { + method() { + stream$.pipe(takeUntil(this._destroy$, 'extraArg')).subscribe(); + } + } + `, + ); + + const clazz = file.getClasses()[0]; + const patterns = (migrator as any).findTakeUntilPatterns(clazz); + + expect(patterns).toHaveLength(0); + }); + }); + + describe("Context Detection Edge Cases", () => { + test("should handle nested constructor calls", () => { + const file = project.createSourceFile( + "test.ts", + ` + import { Component } from '@angular/core'; + import { takeUntil } from 'rxjs/operators'; + + @Component({ selector: 'test' }) + export class TestComponent { + private _destroy$ = new Subject(); + + constructor() { + this.setupStreams(); + } + + private setupStreams() { + stream$.pipe(takeUntil(this._destroy$)).subscribe(); + } + } + `, + ); + + const clazz = file.getClasses()[0]; + const patterns = (migrator as any).findTakeUntilPatterns(clazz); + + expect(patterns).toHaveLength(1); + expect(patterns[0].withinConstructor).toBe(false); + expect(patterns[0].withinMethod).toBe(true); + }); + + test("should handle property initializers", () => { + const file = project.createSourceFile( + "test.ts", + ` + import { Component } from '@angular/core'; + import { takeUntil } from 'rxjs/operators'; + + @Component({ selector: 'test' }) + export class TestComponent { + private _destroy$ = new Subject(); + + private subscription = stream$.pipe(takeUntil(this._destroy$)).subscribe(); + } + `, + ); + + const clazz = file.getClasses()[0]; + const patterns = (migrator as any).findTakeUntilPatterns(clazz); + + expect(patterns).toHaveLength(1); + expect(patterns[0].withinConstructor).toBe(false); + expect(patterns[0].withinMethod).toBe(false); + }); + }); +}); diff --git a/scripts/migrations/takeuntil/tsconfig.json b/scripts/migrations/takeuntil/tsconfig.json new file mode 100644 index 00000000000..364c55d615e --- /dev/null +++ b/scripts/migrations/takeuntil/tsconfig.json @@ -0,0 +1,13 @@ +{ + "compilerOptions": { + "target": "ES2022", + "lib": ["ES2022", "dom"], + "module": "commonjs", + "moduleResolution": "node", + "outDir": "./dist", + "rootDir": ".", + "strict": true, + "types": ["jest", "node"] + }, + "include": ["./test/**/*.ts", "./takeuntil-migrator.ts"] +} diff --git a/scripts/migrations/takeuntil/tsconfig.spec.json b/scripts/migrations/takeuntil/tsconfig.spec.json new file mode 100644 index 00000000000..91690048043 --- /dev/null +++ b/scripts/migrations/takeuntil/tsconfig.spec.json @@ -0,0 +1,13 @@ +{ + "compilerOptions": { + "target": "ES2022", + "lib": ["ES2022"], + "module": "commonjs", + "moduleResolution": "node", + "outDir": "./dist", + "rootDir": ".", + "strict": true, + "types": ["jest", "node"] + }, + "include": ["./test/**/*.ts", "./takeuntil-migrator.ts"] +}