fix(core): missing-injectable migration should not migrate `@NgModule` classes (#36369)

Based on the migration guide, provided classes which don't have
either `@Injectable`, `@Directive`, `@Component` or `@Pipe` need
to be migrated.

This is not correct as provided classes with an `@NgModule` also
have a factory function that can be read by the r3 injector. It's
unclear in which cases the `@NgModule` decorator is used for
provided classes, but this scenario has been reported.

Either we fix this in the migration, or we make sure to report
this as unsupported in the Ivy compiler.

Fixes #35700.

PR Close #36369
This commit is contained in:
Paul Gschwendtner 2020-04-06 15:04:48 +02:00 committed by Matias Niemelä
parent f186c32245
commit 28995dba19
2 changed files with 26 additions and 4 deletions

View File

@ -18,10 +18,14 @@ import {ResolvedDirective, ResolvedNgModule} from './definition_collector';
import {ProviderLiteral, ProvidersEvaluator} from './providers_evaluator'; import {ProviderLiteral, ProvidersEvaluator} from './providers_evaluator';
import {UpdateRecorder} from './update_recorder'; import {UpdateRecorder} from './update_recorder';
/**
* Name of decorators which imply that a given class does not need to be migrated.
/** Name of decorators which imply that a given class does not need to be migrated. */ * - `@Injectable`, `@Directive`, `@Component` and `@Pipe` instruct the compiler
const NO_MIGRATE_DECORATORS = ['Injectable', 'Directive', 'Component', 'Pipe']; * to generate a factory definition.
* - `@NgModule` instructs the compiler to generate a provider definition that holds
* the factory function.
*/
const NO_MIGRATE_DECORATORS = ['Injectable', 'Directive', 'Component', 'Pipe', 'NgModule'];
export interface AnalysisFailure { export interface AnalysisFailure {
node: ts.Node; node: ts.Node;

View File

@ -350,6 +350,24 @@ describe('Missing injectable migration', () => {
expect(tree.readContent('/index.ts')).not.toContain('@Injectable'); expect(tree.readContent('/index.ts')).not.toContain('@Injectable');
}); });
it('should not migrate provider which is already decorated with @NgModule', async () => {
const importedSymbols = type !== 'NgModule' ? ['NgModule', type] : ['NgModule'];
writeFile('/index.ts', `
import {${importedSymbols.join(', ')}} from '@angular/core';
@NgModule()
export class MyOtherModule {}
@${type}({${propName}: [MyOtherModule]})
export class TestClass {}
`);
await runMigration();
expect(warnOutput.length).toBe(0);
expect(tree.readContent('/index.ts')).not.toContain('@Injectable');
});
it(`should migrate multiple providers in same ${type}`, async () => { it(`should migrate multiple providers in same ${type}`, async () => {
writeFile('/index.ts', ` writeFile('/index.ts', `
import {${type}} from '@angular/core'; import {${type}} from '@angular/core';