Skip to content

Commit 2a8cb1e

Browse files
committed
fix(core): migrating input with more than 1 usage in a method
When the migration command was run for inputs, if the input had more than one reference in a method the migration would generate incorrect code Fixes #63018
1 parent e34776a commit 2a8cb1e

File tree

2 files changed

+115
-12
lines changed

2 files changed

+115
-12
lines changed

packages/core/schematics/migrations/signal-migration/src/passes/reference_migration/helpers/standard_reference.ts

Lines changed: 67 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -124,18 +124,44 @@ export function migrateStandardTsReference(
124124
...createNewBlockToInsertVariable(parent, filePath, temporaryVariableStr),
125125
);
126126
} else {
127-
const leadingSpace = ts.getLineAndCharacterOfPosition(sf, referenceNodeInBlock.getStart());
128-
129-
replacements.push(
130-
new Replacement(
131-
filePath,
132-
new TextUpdate({
133-
position: referenceNodeInBlock.getStart(),
134-
end: referenceNodeInBlock.getStart(),
135-
toInsert: `${temporaryVariableStr}\n${' '.repeat(leadingSpace.character)}`,
136-
}),
137-
),
138-
);
127+
// If parent is a Block, insert the temporary variable at the beginning of the block.
128+
if (
129+
ts.isBlock(recommendedNode) &&
130+
shouldInsertAtMethodStart(reference, recommendedNode, referenceNodeInBlock)
131+
) {
132+
const blockStart = recommendedNode.getStart() + 1;
133+
const firstStatement = recommendedNode.statements[0];
134+
const leadingSpace = firstStatement
135+
? ts.getLineAndCharacterOfPosition(sf, firstStatement.getStart())
136+
: ts.getLineAndCharacterOfPosition(sf, referenceNodeInBlock.getStart());
137+
138+
replacements.push(
139+
new Replacement(
140+
filePath,
141+
new TextUpdate({
142+
position: blockStart,
143+
end: blockStart,
144+
toInsert: `\n${' '.repeat(leadingSpace.character)}${temporaryVariableStr}\n${' '.repeat(leadingSpace.character)}`,
145+
}),
146+
),
147+
);
148+
} else {
149+
const leadingSpace = ts.getLineAndCharacterOfPosition(
150+
sf,
151+
referenceNodeInBlock.getStart(),
152+
);
153+
154+
replacements.push(
155+
new Replacement(
156+
filePath,
157+
new TextUpdate({
158+
position: referenceNodeInBlock.getStart(),
159+
end: referenceNodeInBlock.getStart(),
160+
toInsert: `${temporaryVariableStr}\n${' '.repeat(leadingSpace.character)}`,
161+
}),
162+
),
163+
);
164+
}
139165
}
140166

141167
replacements.push(
@@ -151,3 +177,32 @@ export function migrateStandardTsReference(
151177
}
152178
}
153179
}
180+
181+
function shouldInsertAtMethodStart(
182+
references: NarrowableTsReferences,
183+
recommendedNode: ts.Node,
184+
referenceNodeInBlock: ts.Node,
185+
): boolean {
186+
// Check if the recommended node is a method declaration
187+
if (!ts.isBlock(recommendedNode) || !ts.isMethodDeclaration(recommendedNode.parent)) {
188+
return false;
189+
}
190+
191+
// Check all references are within the method body
192+
const methodBody = recommendedNode;
193+
const allReferencesInMethod = references.accesses.every((access) => {
194+
let current: ts.Node | undefined = access;
195+
while (current && current !== methodBody) {
196+
current = current.parent;
197+
}
198+
return current === methodBody;
199+
});
200+
201+
if (!allReferencesInMethod) {
202+
return false;
203+
}
204+
205+
// Verify that referenceNodeInBlock is the first statement of the method
206+
// or at least that no temporary variable has been created before it
207+
return methodBody.statements.length > 0 && methodBody.statements[0] === referenceNodeInBlock;
208+
}

packages/core/schematics/test/signals_migration_spec.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,4 +122,52 @@ describe('combined signals migration', () => {
122122
`),
123123
);
124124
});
125+
126+
it('should be able to migrate input with more than 1 usage in a method', async () => {
127+
writeFile(
128+
'/index.ts',
129+
`
130+
import {Component, Input} from '@angular/core';
131+
132+
@Component({
133+
template: 'it works'
134+
})
135+
export class TestMigrationComponent {
136+
@Input() public model: any;
137+
138+
public onSaveClick(): void {
139+
this.model.requisitionId = 145;
140+
this.model.comment = 'value';
141+
this.model.status = 8;
142+
this.model.finilizeReasonId = 4
143+
}
144+
}`,
145+
);
146+
147+
await runMigration(['inputs']);
148+
149+
expect(stripWhitespace(tree.readContent('/index.ts'))).toBe(
150+
stripWhitespace(
151+
`
152+
import {Component, input} from '@angular/core';
153+
154+
@Component({
155+
template: 'it works'
156+
})
157+
export class TestMigrationComponent {
158+
public readonly model = input<any>(undefined);
159+
160+
public onSaveClick(): void {
161+
const model = this.model();
162+
163+
model.requisitionId = 145;
164+
model.comment = 'value';
165+
model.status = 8;
166+
model.finilizeReasonId = 4
167+
}
168+
}
169+
`,
170+
),
171+
);
172+
});
125173
});

0 commit comments

Comments
 (0)