fix(core): missing-injectable migration should not migrate providers with "useExisting" (#33286)

We should not migrate the reference from `useExisting`. This is because
developers can only use the `useExisting` value as a token. e.g.

```ts
@NgModule({
  providers: [
    {provide: AppRippleConfig, useValue: rippleOptions},
    {provide: MAT_RIPPLE_OPTIONS, useExisting: AppRippleConfig},
  ]
})
export class AppModule {}
```

In the case above, nothing should be decorated with `@Injectable`. The
`AppRippleConfig` class is just used as a token for injection.

PR Close #33286
This commit is contained in:
Paul Gschwendtner
2019-10-22 09:55:39 +02:00
committed by Andrew Kushnir
parent eeecbf28e4
commit 4d23b60d09
5 changed files with 49 additions and 15 deletions

View File

@ -169,12 +169,11 @@ export class MissingInjectableTransform {
if (value instanceof Reference && ts.isClassDeclaration(value.node)) {
this.migrateProviderClass(value.node, module);
} else if (value instanceof Map) {
if (!value.has('provide') || value.has('useValue') || value.has('useFactory')) {
if (!value.has('provide') || value.has('useValue') || value.has('useFactory') ||
value.has('useExisting')) {
return [];
}
if (value.has('useExisting')) {
return this._visitProviderResolvedValue(value.get('useExisting') !, module);
} else if (value.has('useClass')) {
if (value.has('useClass')) {
return this._visitProviderResolvedValue(value.get('useClass') !, module);
} else {
return this._visitProviderResolvedValue(value.get('provide') !, module);

View File

@ -151,7 +151,7 @@ describe('Google3 missing injectable tslint rule', () => {
expect(getFile('/index.ts')).toMatch(/@Injectable\(\)\s+export class MyServiceB/);
expect(getFile('/index.ts')).toMatch(/@Injectable\(\)\s+export class MyServiceC/);
expect(getFile('/index.ts')).toMatch(/@Injectable\(\)\s+export class MyServiceD/);
expect(getFile('/index.ts')).toMatch(/@Injectable\(\)\s+export class MyServiceE/);
expect(getFile('/index.ts')).toMatch(/MyServiceD {}\s+export class MyServiceE/);
expect(getFile('/index.ts')).toMatch(/MyServiceE {}\s+export class MyServiceF/);
expect(getFile('/index.ts')).toMatch(/MyServiceF {}\s+export class MyServiceG/);
});

View File

@ -189,24 +189,24 @@ describe('Missing injectable migration', () => {
expect(tree.readContent('/index.ts')).not.toContain('@Injectable');
});
it(`should migrate object literal provider with "useExisting" in ${type}`, async() => {
it(`should not migrate object literal provider with "useExisting" in ${type}`, async() => {
writeFile('/index.ts', `
import {${type}} from '@angular/core';
export class MyService {}
export class MyToken {}
@${type}({${propName}: [{provide: MyToken, useExisting: MyService}]})
@${type}({${propName}: [
{provide: MyService: useValue: null},
{provide: MyToken, useExisting: MyService},
]})
export class TestClass {}
`);
await runMigration();
expect(warnOutput.length).toBe(0);
expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class MyService/);
expect(tree.readContent('/index.ts')).toMatch(/MyService {}\s+export class MyToken/);
expect(tree.readContent('/index.ts'))
.toContain(`{ ${type}, Injectable } from '@angular/core`);
expect(tree.readContent('/index.ts')).not.toContain('@Injectable');
});
it(`should migrate object literal provider with "useClass" in ${type}`, async() => {
@ -229,6 +229,41 @@ describe('Missing injectable migration', () => {
.toContain(`{ ${type}, Injectable } from '@angular/core`);
});
it(`should not migrate references for providers with "useExisting" in ${type}, but migrate ` +
`existing token if declared in other ${type}`,
async() => {
writeFile('/index.ts', `
import {${type}} from '@angular/core';
export class MyService {}
export class MyToken {}
@${type}({
${propName}: [
{provide: MyToken, useExisting: MyService},
],
})
export class TestClass {}
`);
writeFile('/other.ts', `
import {${type} from '@angular/core';
import {MyService} from './index';
export @${type}({
${propName}: [{provide: MyService}],
})
export class OtherClass {}
`);
await runMigration();
expect(warnOutput.length).toBe(0);
expect(tree.readContent('/index.ts'))
.toMatch(/@angular\/core';\s+@Injectable\(\)\s+export class MyService/);
expect(tree.readContent('/index.ts')).toMatch(/MyService {}\s+export class MyToken/);
});
it('should not migrate provider which is already decorated with @Injectable', async() => {
writeFile('/index.ts', `
import {Injectable, ${type}} from '@angular/core';