Skip to content

Commit

Permalink
fix(core): missing-injectable migration should not update type defini…
Browse files Browse the repository at this point in the history
…tions (#33286)

Currently the `missing-injectable` migration seems to add
`@Injectable()` to third-party classes in type definitions.

This not an issue in general since we do not generate broken code
by inserting a decorator into a type definition file. Though, we can
avoid adding the decorator since it won't have any effect and in
general we should not write to non source files of the compilation unit.

PR Close #33286
  • Loading branch information
devversion authored and AndrewKushnir committed Oct 25, 2019
1 parent 4d23b60 commit 335854f
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
Expand Up @@ -123,6 +123,14 @@ export class MissingInjectableTransform {
this.visitedProviderClasses.add(node);

const sourceFile = node.getSourceFile();

// We cannot migrate provider classes outside of source files. This is because the
// migration for third-party library files should happen in "ngcc", and in general
// would also involve metadata parsing.
if (sourceFile.isDeclarationFile) {
return;
}

const ngDecorators =
node.decorators ? getAngularDecorators(this.typeChecker, node.decorators) : null;

Expand Down
Expand Up @@ -380,7 +380,6 @@ describe('Missing injectable migration', () => {
.toContain(`{ ${type}, Injectable } from '@angular/core`);
});


it(`should migrate multiple nested providers in same ${type}`, async() => {
writeFile('/index.ts', `
import {${type}} from '@angular/core';
Expand Down Expand Up @@ -689,5 +688,24 @@ describe('Missing injectable migration', () => {
expect(tree.readContent('/service.ts'))
.toMatch(/import { Inject, Injectable } from '@angular\/core';/);
});

it('should not migrate provider classes in library type definitions', async() => {
writeFile('/node_modules/my-lib/index.d.ts', `
export declare class MyService {}
`);

writeFile('/index.ts', `
import {MyService} from 'my-lib';
import {Pipe, ${type}} from '@angular/core';
@${type}({${propName}: [MyService]})
export class TestClass {}
`);

await runMigration();

expect(warnOutput.length).toBe(0);
expect(tree.readContent('/node_modules/my-lib/index.d.ts')).not.toContain('@Injectable');
});
}
});

0 comments on commit 335854f

Please sign in to comment.