diff --git a/packages/core/schematics/migrations/signal-migration/src/flow_analysis/index.ts b/packages/core/schematics/migrations/signal-migration/src/flow_analysis/index.ts index 5586ec044bf..03a4a02b235 100644 --- a/packages/core/schematics/migrations/signal-migration/src/flow_analysis/index.ts +++ b/packages/core/schematics/migrations/signal-migration/src/flow_analysis/index.ts @@ -87,10 +87,18 @@ export function analyzeControlFlow( // Prepare easy lookups for reference nodes to flow info. for (const [idx, entry] of entries.entries()) { + const flowContainer = getControlFlowContainer(entry); referenceToMetadata.set(entry, { - flowContainer: getControlFlowContainer(entry), + flowContainer, resultIndex: idx, }); + + result.push({ + flowContainer, + id: idx, + originalNode: entry, + recommendedNode: 'preserve', + }); } for (const entry of entries) { @@ -110,13 +118,6 @@ export function analyzeControlFlow( checker, ); - result.push({ - id: resultIndex, - originalNode: entry, - flowContainer, - recommendedNode: 'preserve', - }); - if (narrowPartners.length !== 0) { connectSharedReferences(result, narrowPartners, resultIndex); } @@ -154,13 +155,19 @@ function connectSharedReferences( assert(earliestPartner !== null, 'Expected an earliest partner to be found.'); assert(earliestPartnerId !== null, 'Expected an earliest partner to be found.'); + // Earliest partner ID could be higher than `refId` in cyclic + // situations like `loop` flow nodes. We need to find the minimum + // and maximum to iterate through partners in between. + const min = Math.min(earliestPartnerId, refId); + const max = Math.max(earliestPartnerId, refId); + // Then, incorporate all similar references (or flow nodes) in between // the reference and the earliest partner. References in between can also // use the shared flow node and not preserve their original reference— as // this would be rather unreadable and inefficient. const seenBlocks = new Set(); let highestBlock: ts.Block | ts.SourceFile | ts.ArrowFunction | null = null; - for (let i = earliestPartnerId; i <= refId; i++) { + for (let i = min; i <= max; i++) { // Different flow container captured sequentially in result. Ignore. if (result[i].flowContainer !== refFlowContainer) { continue; diff --git a/packages/core/schematics/migrations/signal-migration/src/passes/10_apply_import_manager.ts b/packages/core/schematics/migrations/signal-migration/src/passes/10_apply_import_manager.ts index 14ac6c789e8..5587b45c2c7 100644 --- a/packages/core/schematics/migrations/signal-migration/src/passes/10_apply_import_manager.ts +++ b/packages/core/schematics/migrations/signal-migration/src/passes/10_apply_import_manager.ts @@ -10,7 +10,6 @@ import {ImportManager} from '@angular/compiler-cli/src/ngtsc/translator'; import ts from 'typescript'; import {applyImportManagerChanges} from '../../../../utils/tsurge/helpers/apply_import_manager'; import {MigrationResult} from '../result'; -import {AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/file_system'; import {ProgramInfo} from '../../../../utils/tsurge'; /** diff --git a/packages/core/schematics/migrations/signal-migration/src/passes/reference_migration/helpers/standard_reference.ts b/packages/core/schematics/migrations/signal-migration/src/passes/reference_migration/helpers/standard_reference.ts index 05ff4a1f847..c34488e4b62 100644 --- a/packages/core/schematics/migrations/signal-migration/src/passes/reference_migration/helpers/standard_reference.ts +++ b/packages/core/schematics/migrations/signal-migration/src/passes/reference_migration/helpers/standard_reference.ts @@ -7,11 +7,12 @@ */ import ts from 'typescript'; -import {analyzeControlFlow} from '../../../flow_analysis'; +import {analyzeControlFlow, ControlFlowAnalysisNode} from '../../../flow_analysis'; import {ProgramInfo, projectFile, Replacement, TextUpdate} from '../../../../../../utils/tsurge'; import {traverseAccess} from '../../../utils/traverse_access'; import {UniqueNamesGenerator} from '../../../utils/unique_names'; import {createNewBlockToInsertVariable} from '../helpers/create_block_arrow_function'; +import assert from 'assert'; export interface NarrowableTsReferences { accesses: ts.Identifier[]; @@ -30,6 +31,19 @@ export function migrateStandardTsReference( const controlFlowResult = analyzeControlFlow(reference.accesses, checker); const idToSharedField = new Map(); + const isSharePartnerRef = (val: ControlFlowAnalysisNode['recommendedNode']) => { + return val !== 'preserve' && typeof val !== 'number'; + }; + + // Ensure we generate shared fields before reference entries. + // This allows us to safely make use of `idToSharedField` whenever we come + // across a referenced pointing to a share partner. + controlFlowResult.sort((a, b) => { + const aPriority = isSharePartnerRef(a.recommendedNode) ? 1 : 0; + const bPriority = isSharePartnerRef(b.recommendedNode) ? 1 : 0; + return bPriority - aPriority; + }); + for (const {id, originalNode, recommendedNode} of controlFlowResult) { const sf = originalNode.getSourceFile(); @@ -53,15 +67,18 @@ export function migrateStandardTsReference( // This reference is shared with a previous reference. Replace the access // with the temporary variable. if (typeof recommendedNode === 'number') { + // Extract the shared field name. + const toInsert = idToSharedField.get(recommendedNode); const replaceNode = traverseAccess(originalNode); + + assert(toInsert, 'no shared variable yet available'); replacements.push( new Replacement( projectFile(sf, info), new TextUpdate({ position: replaceNode.getStart(), end: replaceNode.getEnd(), - // Extract the shared field name. - toInsert: idToSharedField.get(recommendedNode)!, + toInsert, }), ), ); diff --git a/packages/core/schematics/migrations/signal-migration/test/golden-test/flow_cases.ts b/packages/core/schematics/migrations/signal-migration/test/golden-test/flow_cases.ts new file mode 100644 index 00000000000..e2c6644b352 --- /dev/null +++ b/packages/core/schematics/migrations/signal-migration/test/golden-test/flow_cases.ts @@ -0,0 +1,13 @@ +// tslint:disable + +import {Input} from '@angular/core'; + +class Test { + @Input() maxCellsPerRow = 5; + + private test(arr: string[]) { + for (let i = 0; i < arr.length; i += this.maxCellsPerRow) { + console.log(this.maxCellsPerRow); + } + } +} diff --git a/packages/core/schematics/migrations/signal-migration/test/golden.txt b/packages/core/schematics/migrations/signal-migration/test/golden.txt index c532d602083..55dbfdfd022 100644 --- a/packages/core/schematics/migrations/signal-migration/test/golden.txt +++ b/packages/core/schematics/migrations/signal-migration/test/golden.txt @@ -368,6 +368,22 @@ import {Component, input} from '@angular/core'; export class WithTemplate { readonly test = input(true); } +@@@@@@ flow_cases.ts @@@@@@ + +// tslint:disable + +import {input} from '@angular/core'; + +class Test { + readonly maxCellsPerRow = input(5); + + private test(arr: string[]) { + const maxCellsPerRow = this.maxCellsPerRow(); + for (let i = 0; i < arr.length; i += maxCellsPerRow) { + console.log(maxCellsPerRow); + } + } +} @@@@@@ getters.ts @@@@@@ // tslint:disable diff --git a/packages/core/schematics/migrations/signal-migration/test/golden_best_effort.txt b/packages/core/schematics/migrations/signal-migration/test/golden_best_effort.txt index 86564229c7a..657e128f4bf 100644 --- a/packages/core/schematics/migrations/signal-migration/test/golden_best_effort.txt +++ b/packages/core/schematics/migrations/signal-migration/test/golden_best_effort.txt @@ -344,6 +344,22 @@ import {Component, input} from '@angular/core'; export class WithTemplate { readonly test = input(true); } +@@@@@@ flow_cases.ts @@@@@@ + +// tslint:disable + +import {input} from '@angular/core'; + +class Test { + readonly maxCellsPerRow = input(5); + + private test(arr: string[]) { + const maxCellsPerRow = this.maxCellsPerRow(); + for (let i = 0; i < arr.length; i += maxCellsPerRow) { + console.log(maxCellsPerRow); + } + } +} @@@@@@ getters.ts @@@@@@ // tslint:disable