From ddfaf0cd4684d111b875f9a456aaa98a166d1c01 Mon Sep 17 00:00:00 2001 From: Matthieu Riegler Date: Sun, 23 Feb 2025 01:26:33 +0100 Subject: [PATCH] refactor(migrations): Don't migrate twice the same file on the self-closing tag migration (#60065) This commit fixes an issue when ts files are referenced multiple times (and thus analyzed multiple times) for example from a `tsconfig.json` and `tsconfig.spec.json`. PR Close #60065 --- .../self-closing-tags-migration.ts | 63 +++++++++++++------ .../test/self_closing_tags_migration_spec.ts | 37 +++++++++++ 2 files changed, 80 insertions(+), 20 deletions(-) diff --git a/packages/core/schematics/migrations/self-closing-tags-migration/self-closing-tags-migration.ts b/packages/core/schematics/migrations/self-closing-tags-migration/self-closing-tags-migration.ts index cc36690f520..e3ffe0f5e10 100644 --- a/packages/core/schematics/migrations/self-closing-tags-migration/self-closing-tags-migration.ts +++ b/packages/core/schematics/migrations/self-closing-tags-migration/self-closing-tags-migration.ts @@ -56,6 +56,7 @@ export class SelfClosingTagsMigration extends TsurgeFunnelMigration< for (const sf of sourceFiles) { ts.forEachChild(sf, (node: ts.Node) => { + // Skipping any non component declarations if (!ts.isClassDeclaration(node)) { return; } @@ -75,26 +76,28 @@ export class SelfClosingTagsMigration extends TsurgeFunnelMigration< template.content, ); - if (changed) { - const fileToMigrate = template.inline - ? file - : projectFile(template.filePath as AbsoluteFsPath, info); - const end = template.start + template.content.length; + if (!changed) { + return; + } - const replacements = [ - prepareTextReplacement(fileToMigrate, migrated, template.start, end), - ]; + const fileToMigrate = template.inline + ? file + : projectFile(template.filePath as AbsoluteFsPath, info); + const end = template.start + template.content.length; - const fileReplacements = tagReplacements.find( - (tagReplacement) => tagReplacement.file === file, - ); + const replacements = [ + prepareTextReplacement(fileToMigrate, migrated, template.start, end), + ]; - if (fileReplacements) { - fileReplacements.replacements.push(...replacements); - fileReplacements.replacementCount += replacementCount; - } else { - tagReplacements.push({file, replacements, replacementCount}); - } + const fileReplacements = tagReplacements.find( + (tagReplacement) => tagReplacement.file === file, + ); + + if (fileReplacements) { + fileReplacements.replacements.push(...replacements); + fileReplacements.replacementCount += replacementCount; + } else { + tagReplacements.push({file, replacements, replacementCount}); } }); }); @@ -107,9 +110,12 @@ export class SelfClosingTagsMigration extends TsurgeFunnelMigration< unitA: SelfClosingTagsCompilationUnitData, unitB: SelfClosingTagsCompilationUnitData, ): Promise> { - return confirmAsSerializable({ - tagReplacements: unitA.tagReplacements.concat(unitB.tagReplacements), - }); + const uniqueReplacements = removeDuplicateReplacements([ + ...unitA.tagReplacements, + ...unitB.tagReplacements, + ]); + + return confirmAsSerializable({tagReplacements: uniqueReplacements}); } override async globalMeta( @@ -159,3 +165,20 @@ function prepareTextReplacement( }), ); } + +function removeDuplicateReplacements( + replacements: SelfClosingTagsMigrationData[], +): SelfClosingTagsMigrationData[] { + const uniqueFiles = new Set(); + const result: SelfClosingTagsMigrationData[] = []; + + for (const replacement of replacements) { + const fileId = replacement.file.id; + if (!uniqueFiles.has(fileId)) { + uniqueFiles.add(fileId); + result.push(replacement); + } + } + + return result; +} diff --git a/packages/core/schematics/test/self_closing_tags_migration_spec.ts b/packages/core/schematics/test/self_closing_tags_migration_spec.ts index 931dc501387..23740845a90 100644 --- a/packages/core/schematics/test/self_closing_tags_migration_spec.ts +++ b/packages/core/schematics/test/self_closing_tags_migration_spec.ts @@ -19,6 +19,7 @@ describe('self-closing-tags migration', () => { let tree: UnitTestTree; let tmpDirPath: string; let previousWorkingDir: string; + let logs: string[]; function writeFile(filePath: string, contents: string) { host.sync.write(normalize(filePath), virtualFs.stringToFileBuffer(contents)); @@ -32,6 +33,7 @@ describe('self-closing-tags migration', () => { runner = new SchematicTestRunner('test', runfiles.resolvePackageRelative('../collection.json')); host = new TempScopedNodeJsSyncHost(); tree = new UnitTestTree(new HostTree(host)); + logs = []; writeFile('/tsconfig.json', '{}'); writeFile( @@ -44,6 +46,7 @@ describe('self-closing-tags migration', () => { previousWorkingDir = shx.pwd(); tmpDirPath = getSystemPath(host.root); + runner.logger.subscribe((log) => logs.push(log.message)); shx.cd(tmpDirPath); }); @@ -66,5 +69,39 @@ describe('self-closing-tags migration', () => { const content = tree.readContent('/app.component.ts').replace(/\s+/g, ' '); expect(content).toContain(''); + expect(logs.pop()).toBe( + ' -> Migrated 1 components to self-closing tags in 1 component files.', + ); + }); + + it('should handle a file that is present in multiple projects', async () => { + writeFile('/tsconfig-2.json', '{}'); + writeFile( + '/angular.json', + JSON.stringify({ + version: 1, + projects: { + a: {root: '', architect: {build: {options: {tsConfig: './tsconfig.json'}}}}, + b: {root: '', architect: {build: {options: {tsConfig: './tsconfig-2.json'}}}}, + }, + }), + ); + + writeFile( + '/app.component.ts', + ` + import {Component} from '@angular/core'; + @Component({ template: '' }) + export class Cmp {} + `, + ); + + await runMigration(); + + const content = tree.readContent('/app.component.ts').replace(/\s+/g, ' '); + expect(content).toContain(''); + expect(logs.pop()).toBe( + ' -> Migrated 1 components to self-closing tags in 1 component files.', + ); }); });