cargdev/angular
1
0
Fork 0
Code Issues 8 Pull Requests Actions Packages Projects Releases Wiki Activity

feat(core): missing-injectable migration should migrate empty object literal providers (#33709)

Browse Source 
In View Engine, providers which neither used `useValue`, `useClass`,
`useFactory` or `useExisting`, were interpreted differently.

e.g.

```
{provide: X} -> {provide: X, useValue: undefined}, // this is how it works in View Engine
{provide: X} -> {provide: X, useClass: X}, // this is how it works in Ivy
```

The missing-injectable migration should migrate such providers to the
explicit `useValue` provider. This ensures that there is no unexpected
behavioral change when updating to v9.

PR Close #33709
This commit is contained in:
Paul Gschwendtner
2019-11-14 10:16:07 +01:00
committed by Alex Rickabaugh
parent 1b7aa05699
commit b7c012f91b
10 changed files with 229 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
packages/core/schematics/test/google3/missing_injectable_rule_spec.ts
View File

@ -121,6 +121,7 @@ describe('Google3 missing injectable tslint rule', () => {
export class MyServiceE {}
export class MyServiceF {}
export class MyServiceG {}
export class MyServiceH {}
@${type}({${propName}: [
WithPipe,
@ -135,6 +136,7 @@ describe('Google3 missing injectable tslint rule', () => {
{provide: null, useExisting: MyServiceE},
{provide: MyServiceF, useFactory: () => null},
{provide: MyServiceG, useValue: null},
{provide: MyServiceH, deps: []},
]})
export class TestClass {}
`);
@ -149,11 +151,13 @@ describe('Google3 missing injectable tslint rule', () => {
.toMatch(/WithDirective {}\s+@Component\(\)\s+export class WithComponent/);
expect(getFile('/index.ts')).toMatch(/@Injectable\(\)\s+export class MyServiceA/);
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(/MyServiceB {}\s+export class MyServiceC/);
expect(getFile('/index.ts')).toMatch(/@Injectable\(\)\s+export class MyServiceD/);
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/);
expect(getFile('/index.ts')).toMatch(/MyServiceG {}\s+export class MyServiceH/);
expect(getFile('/index.ts')).toContain(`{ provide: MyServiceC, useValue: undefined },`);
});
it(`should migrate provider once if referenced in multiple ${type} definitions`, () => {

101
packages/core/schematics/test/missing_injectable_migration_spec.ts
View File

@ -121,7 +121,7 @@ describe('Missing injectable migration', () => {
.toContain(`{ ${type}, Injectable } from '@angular/core`);
});
it(`should migrate object literal provider in ${type}`, async() => {
it(`should migrate object literal provider in ${type} to explicit value provider`, async() => {
writeFile('/index.ts', `
import {${type}} from '@angular/core';
@ -134,16 +134,17 @@ describe('Missing injectable migration', () => {
await runMigration();
expect(warnOutput.length).toBe(0);
expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class MyService/);
expect(tree.readContent('/index.ts')).toMatch(/@angular\/core';\s+export class MyService/);
expect(tree.readContent('/index.ts'))
.toContain(`{ ${type}, Injectable } from '@angular/core`);
.toContain(`${propName}: [{ provide: MyService, useValue: undefined }]`);
expect(tree.readContent('/index.ts')).toContain(`{${type}} from '@angular/core`);
});
it(`should migrate object literal provider with forwardRef in ${type}`, async() => {
writeFile('/index.ts', `
import {${type}, forwardRef} from '@angular/core';
@${type}({${propName}: [{provide: forwardRef(() => MyService)}]})
@${type}({${propName}: [forwardRef(() => MyService)]})
export class TestClass {}
export class MyService {}
@ -173,6 +174,22 @@ describe('Missing injectable migration', () => {
expect(tree.readContent('/index.ts')).not.toContain('@Injectable');
});
it(`should not migrate provider with "useClass" and "deps" in ${type}`, async() => {
writeFile('/index.ts', `
import {${type}} from '@angular/core';
export class MyService {}
@${type}({${propName}: [{provide: MyService, deps: []}]})
export class TestClass {}
`);
await runMigration();
expect(warnOutput.length).toBe(0);
expect(tree.readContent('/index.ts')).not.toContain('@Injectable');
});
it(`should not migrate object literal provider with "useFactory" in ${type}`, async() => {
writeFile('/index.ts', `
import {${type}} from '@angular/core';
@ -251,7 +268,7 @@ describe('Missing injectable migration', () => {
import {MyService} from './index';
export @${type}({
${propName}: [{provide: MyService}],
${propName}: [{provide: MyService, useClass: MyService}],
})
export class OtherClass {}
`);
@ -374,10 +391,12 @@ describe('Missing injectable migration', () => {
expect(warnOutput.length).toBe(0);
expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceA/);
expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceB/);
expect(tree.readContent('/index.ts')).toMatch(/ServiceA {}\s+export class ServiceB/);
expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceC/);
expect(tree.readContent('/index.ts'))
.toContain(`{ ${type}, Injectable } from '@angular/core`);
expect(tree.readContent('/index.ts'))
.toContain(`{ provide: ServiceB, useValue: undefined },`);
});
it(`should migrate multiple nested providers in same ${type}`, async() => {
@ -387,13 +406,15 @@ describe('Missing injectable migration', () => {
export class ServiceA {}
export class ServiceB {}
export class ServiceC {}
export class ServiceD {}
@${type}({
${propName}: [
ServiceA,
[
{provide: ServiceB},
{provide: ServiceB, useClass: ServiceB},
ServiceC,
{provide: ServiceD},
],
]})
export class TestClass {}
@ -405,8 +426,11 @@ describe('Missing injectable migration', () => {
expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceA/);
expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceB/);
expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceC/);
expect(tree.readContent('/index.ts')).toMatch(/ServiceC {}\s+export class ServiceD/);
expect(tree.readContent('/index.ts'))
.toContain(`{ ${type}, Injectable } from '@angular/core`);
expect(tree.readContent('/index.ts'))
.toContain(`{ provide: ServiceD, useValue: undefined },`);
});
it('should migrate providers referenced through identifier', async() => {
@ -415,8 +439,15 @@ describe('Missing injectable migration', () => {
export class ServiceA {}
export class ServiceB {}
export class ServiceC {}
const PROVIDERS = [ServiceA, ServiceB];
const PROVIDERS = [
ServiceA,
ServiceB,
// trailing comma is by intention to check if do not create
// an invalid object literal.
{provide: ServiceC, },
];
@${type}({
${propName}: PROVIDERS,
@ -429,8 +460,11 @@ describe('Missing injectable migration', () => {
expect(warnOutput.length).toBe(0);
expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceA/);
expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceB/);
expect(tree.readContent('/index.ts')).toMatch(/ServiceB {}\s+export class ServiceC/);
expect(tree.readContent('/index.ts'))
.toContain(`{ ${type}, Injectable } from '@angular/core`);
expect(tree.readContent('/index.ts'))
.toContain(`{ provide: ServiceC, useValue: undefined },`);
});
it('should migrate providers created through static analyzable function call', async() => {
@ -439,13 +473,14 @@ describe('Missing injectable migration', () => {
export class ServiceA {}
export class ServiceB {}
export class ServiceC {}
export function createProviders(x: any) {
return [ServiceA, x]
export function createProviders(x: any, b: any) {
return [ServiceA, x, b]
}
@${type}({
${propName}: createProviders(ServiceB),
${propName}: createProviders(ServiceB, {provide: ServiceC}),
})
export class TestClass {}
`);
@ -455,8 +490,11 @@ describe('Missing injectable migration', () => {
expect(warnOutput.length).toBe(0);
expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceA/);
expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceB/);
expect(tree.readContent('/index.ts')).toMatch(/ServiceB {}\s+export class ServiceC/);
expect(tree.readContent('/index.ts'))
.toContain(`{ ${type}, Injectable } from '@angular/core`);
expect(tree.readContent('/index.ts'))
.toContain(`ServiceB, { provide: ServiceC, useValue: undefined }),`);
});
it('should migrate providers which are computed through spread operator', async() => {
@ -465,8 +503,9 @@ describe('Missing injectable migration', () => {
export class ServiceA {}
export class ServiceB {}
export class ServiceC {}
const otherServices = [ServiceB];
const otherServices = [ServiceB, {provide: ServiceC}];
@${type}({
${propName}: [ServiceA, ...otherServices],
@ -479,8 +518,11 @@ describe('Missing injectable migration', () => {
expect(warnOutput.length).toBe(0);
expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceA/);
expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceB/);
expect(tree.readContent('/index.ts')).toMatch(/ServiceB {}\s+export class ServiceC/);
expect(tree.readContent('/index.ts'))
.toContain(`{ ${type}, Injectable } from '@angular/core`);
expect(tree.readContent('/index.ts'))
.toContain(`ServiceB, { provide: ServiceC, useValue: undefined }];`);
});
it(`should migrate provider once if referenced in multiple ${type} definitions`, async() => {
@ -515,6 +557,41 @@ describe('Missing injectable migration', () => {
.toContain(`{ ${type}, Injectable } from '@angular/core`);
});
it(`should only migrate empty object provider literal once if referenced multiple times ` +
`in ${type} definitions`,
async() => {
writeFile('/provider.ts', `
export class MyService {}
export const PROVIDER = {provide: MyService};
`);
writeFile('/index.ts', `
import {${type}} from '@angular/core';
import {PROVIDER} from './provider';
@${type}({${propName}: [PROVIDER]})
export class TestClassA {}
`);
writeFile('/second.ts', `
import {${type}} from '@angular/core';
import {PROVIDER} from './provider';
export class ServiceB {}
@${type}({${propName}: [PROVIDER, ServiceB]})
export class TestClassB {}
`);
await runMigration();
expect(warnOutput.length).toBe(0);
expect(tree.readContent('/provider.ts')).toMatch(/^\s+export class MyService {}/);
expect(tree.readContent('/provider.ts'))
.toContain(`const PROVIDER = { provide: MyService, useValue: undefined };`);
});
it('should create new import for @Injectable when migrating provider', async() => {
writeFile('/index.ts', `
import {${type}} from '@angular/core';