Skip to content

Commit 3ff48b6

Browse files
crisbetothePunderWoman
authored andcommitted
fix(migrations): inject migration not handling super parameter referenced via this (angular#60602)
The inject migration has some logic that treats parameters referenced directly inside of `super` differently. This logic didn't account for the fact that the parameters could be inside of inline functions which have less strict access requirements. PR Close angular#60602
1 parent 7c97156 commit 3ff48b6

File tree

3 files changed

+49
-7
lines changed

3 files changed

+49
-7
lines changed

packages/core/schematics/ng-generate/inject-migration/analysis.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,9 @@ export function getSuperParameters(
304304
usedParams.add(decl);
305305
}
306306
});
307-
} else {
307+
// Parameters referenced inside callbacks can be used directly
308+
// within `super` so don't descend into inline functions.
309+
} else if (!isInlineFunction(node)) {
308310
node.forEachChild(walk);
309311
}
310312
});
@@ -423,3 +425,12 @@ function findSuperCall(root: ts.Node): ts.CallExpression | null {
423425

424426
return result;
425427
}
428+
429+
/** Checks whether a node is an inline function. */
430+
export function isInlineFunction(
431+
node: ts.Node,
432+
): node is ts.FunctionDeclaration | ts.FunctionExpression | ts.ArrowFunction {
433+
return (
434+
ts.isFunctionDeclaration(node) || ts.isFunctionExpression(node) || ts.isArrowFunction(node)
435+
);
436+
}

packages/core/schematics/ng-generate/inject-migration/internal.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import ts from 'typescript';
10-
import {isAccessedViaThis, parameterDeclaresProperty} from './analysis';
10+
import {isAccessedViaThis, isInlineFunction, parameterDeclaresProperty} from './analysis';
1111

1212
/** Property that is a candidate to be combined. */
1313
interface CombineCandidate {
@@ -299,11 +299,7 @@ function isInsideInlineFunction(startNode: ts.Node, boundary: ts.Node): boolean
299299
return false;
300300
}
301301

302-
if (
303-
ts.isFunctionDeclaration(current) ||
304-
ts.isFunctionExpression(current) ||
305-
ts.isArrowFunction(current)
306-
) {
302+
if (isInlineFunction(current)) {
307303
return true;
308304
}
309305

packages/core/schematics/test/inject_migration_spec.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1800,6 +1800,41 @@ describe('inject migration', () => {
18001800
]);
18011801
});
18021802

1803+
it('should handle parameter referenced through `this` inside a callback within super()', async () => {
1804+
writeFile(
1805+
'/dir.ts',
1806+
[
1807+
`import { Directive } from '@angular/core';`,
1808+
`import { Service } from './service';`,
1809+
`import { Parent } from './parent';`,
1810+
``,
1811+
`@Directive()`,
1812+
`export class MyDir extends Parent {`,
1813+
` constructor(private service: Service) {`,
1814+
` super({callback: () => this.service.doStuff()});`,
1815+
` }`,
1816+
`}`,
1817+
].join('\n'),
1818+
);
1819+
1820+
await runMigration();
1821+
1822+
expect(tree.readContent('/dir.ts').split('\n')).toEqual([
1823+
`import { Directive, inject } from '@angular/core';`,
1824+
`import { Service } from './service';`,
1825+
`import { Parent } from './parent';`,
1826+
``,
1827+
`@Directive()`,
1828+
`export class MyDir extends Parent {`,
1829+
` private service = inject(Service);`,
1830+
``,
1831+
` constructor() {`,
1832+
` super({callback: () => this.service.doStuff()});`,
1833+
` }`,
1834+
`}`,
1835+
]);
1836+
});
1837+
18031838
describe('internal-only behavior', () => {
18041839
function runInternalMigration() {
18051840
return runMigration({_internalCombineMemberInitializers: true});

0 commit comments

Comments
 (0)