diff --git a/packages/core/schematics/migrations/http-providers/README.md b/packages/core/schematics/migrations/http-providers/README.md index 4eaa3bb7e9e7a1..b16718aa3f6227 100644 --- a/packages/core/schematics/migrations/http-providers/README.md +++ b/packages/core/schematics/migrations/http-providers/README.md @@ -13,7 +13,7 @@ This migration updates any `@NgModule`, `@Component`, `@Directive` that imports import { HttpClientModule, HttpClientJsonpModule, HttpClientXsrfModule } from '@angular/common/http'; @NgModule({ - imports: [CommonModule, HttpClientModule,HttpClientJsonpModule, HttpClientXsrfModule)], + imports: [CommonModule, HttpClientModule, HttpClientJsonpModule, HttpClientXsrfModule] }) export class AppModule {} ``` @@ -33,30 +33,61 @@ export class AppModule {} #### Before +```ts +import { HttpClientTestingModule } from '@angular/common/http/testing'; + +describe('some test', () => { + + it('...', () => { + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule] + }) + }) +}) ``` -import { HttpClientTestingModule } from '@not-angular/common/http/testing'; -describe('some test') { +#### After + +```ts +import { provideHttpClientTesting } from '@angular/common/http/testing'; +import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http'; + +describe('some test', () => { it('...', () => { TestBed.configureTestingModule({ - imports: [HttpClientTestingModule], - }); + providers: [provideHttpClient(withInterceptorsFromDi()), provideHttpClientTesting()] + }) }) -} +}) ``` #### Before +```ts +import { HttpClientTesting } from '@angular/common/http'; + +describe('some test', () => { + + it('...', () => { + TestBed.configureTestingModule({ + imports: [HttpClientTesting], + }) + }) +}); ``` -import { provideHttpClientTesting } from '@not-angular/common/http/testing'; -describe('some test') { +#### After + +```ts +import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http'; + +describe('some test', () => { it('...', () => { TestBed.configureTestingModule({ - providers: [provideHttpClientTesting()], - }); + providers: [provideHttpClient(withInterceptorsFromDi())] + }) }) -} +}) ``` diff --git a/packages/core/schematics/migrations/http-providers/utils.ts b/packages/core/schematics/migrations/http-providers/utils.ts index 5c653377df6326..384c40337e3cea 100644 --- a/packages/core/schematics/migrations/http-providers/utils.ts +++ b/packages/core/schematics/migrations/http-providers/utils.ts @@ -67,7 +67,13 @@ export function migrateFile( }); } - migrateTestingModuleImports(node, commonHttpTestingIdentifiers, addedImports, changeTracker); + migrateTestingModuleImports( + node, + commonHttpIdentifiers, + commonHttpTestingIdentifiers, + addedImports, + changeTracker, + ); }); // Imports are for the whole file @@ -90,6 +96,13 @@ export function migrateFile( ]); changeTracker.replaceNode(commonHttpImports, newImports); } + // If there are no imports for common/http, and we need to add some + else if (addedImports.get(COMMON_HTTP)?.size) { + // Then we add a new import statement for common/http + addedImports.get(COMMON_HTTP)?.forEach((entry) => { + changeTracker.addImport(sourceFile, entry, COMMON_HTTP); + }); + } // Remove the HttpModules imports from common/http/testing const commonHttpTestingImports = getNamedImports(sourceFile, COMMON_HTTP_TESTING); @@ -198,7 +211,7 @@ function migrateDecorator( let newProviders: ts.ArrayLiteralExpression; if (!providers) { - // No existing providers, we add an property to the literal + // No existing providers, we add a property to the literal newProviders = ts.factory.createArrayLiteralExpression([provideHttpExpr]); } else { // We add the provider to the existing provider array @@ -219,6 +232,7 @@ function migrateDecorator( function migrateTestingModuleImports( node: ts.Node, + commonHttpIdentifiers: Set, commonHttpTestingIdentifiers: Set, addedImports: Map>, changeTracker: ChangeTracker, @@ -248,51 +262,91 @@ function migrateTestingModuleImports( return; } + // Does the imports array contain the HttpClientModule? + const httpClient = importsArray.elements.find((elt) => elt.getText() === HTTP_CLIENT_MODULE); + if (httpClient && commonHttpIdentifiers.has(HTTP_CLIENT_MODULE)) { + // We add the imports for provideHttpClient(withInterceptorsFromDi()) + addedImports.get(COMMON_HTTP)?.add(PROVIDE_HTTP_CLIENT); + addedImports.get(COMMON_HTTP)?.add(WITH_INTERCEPTORS_FROM_DI); + + const newImports = ts.factory.createArrayLiteralExpression([ + ...importsArray.elements.filter((item) => item !== httpClient), + ]); + + const provideHttpClient = createCallExpression(PROVIDE_HTTP_CLIENT, [ + createCallExpression(WITH_INTERCEPTORS_FROM_DI), + ]); + + // Adding the new provider + const providers = getProvidersFromLiteralExpr(configureTestingModuleArgs); + + let newProviders: ts.ArrayLiteralExpression; + if (!providers) { + // No existing providers, we add a property to the literal + newProviders = ts.factory.createArrayLiteralExpression([provideHttpClient]); + } else { + // We add the provider to the existing provider array + newProviders = ts.factory.createArrayLiteralExpression([ + ...providers.elements, + provideHttpClient, + ]); + } + + // Replacing the existing configuration with the new one (with the new imports and providers) + const newTestingModuleArgs = ts.factory.createObjectLiteralExpression([ + ...configureTestingModuleArgs.properties.filter((p) => p.getText() === 'imports'), + ts.factory.createPropertyAssignment('imports', newImports), + ts.factory.createPropertyAssignment('providers', newProviders), + ]); + changeTracker.replaceNode(configureTestingModuleArgs, newTestingModuleArgs); + } + // Does the imports array contain the HttpClientTestingModule? const httpClientTesting = importsArray.elements.find( (elt) => elt.getText() === HTTP_CLIENT_TESTING_MODULE, ); - if (!httpClientTesting || !commonHttpTestingIdentifiers.has(HTTP_CLIENT_TESTING_MODULE)) { - return; - } - - addedImports.get(COMMON_HTTP_TESTING)?.add(PROVIDE_HTTP_CLIENT_TESTING); - - const newImports = ts.factory.createArrayLiteralExpression([ - ...importsArray.elements.filter((item) => item !== httpClientTesting), - ]); - - const provideHttpClient = createCallExpression(PROVIDE_HTTP_CLIENT, [ - createCallExpression(WITH_INTERCEPTORS_FROM_DI), - ]); - const provideHttpClientTesting = createCallExpression(PROVIDE_HTTP_CLIENT_TESTING); + if (httpClientTesting && commonHttpTestingIdentifiers.has(HTTP_CLIENT_TESTING_MODULE)) { + // We add the imports for provideHttpClient(withInterceptorsFromDi()) and provideHttpClientTesting() + addedImports.get(COMMON_HTTP)?.add(PROVIDE_HTTP_CLIENT); + addedImports.get(COMMON_HTTP)?.add(WITH_INTERCEPTORS_FROM_DI); + addedImports.get(COMMON_HTTP_TESTING)?.add(PROVIDE_HTTP_CLIENT_TESTING); - // Adding the new providers - const providers = getProvidersFromLiteralExpr(configureTestingModuleArgs); + const newImports = ts.factory.createArrayLiteralExpression([ + ...importsArray.elements.filter((item) => item !== httpClientTesting), + ]); - let newProviders: ts.ArrayLiteralExpression; - if (!providers) { - // No existing providers, we add an property to the literal - newProviders = ts.factory.createArrayLiteralExpression([ - provideHttpClient, - provideHttpClientTesting, + const provideHttpClient = createCallExpression(PROVIDE_HTTP_CLIENT, [ + createCallExpression(WITH_INTERCEPTORS_FROM_DI), ]); - } else { - // We add the provider to the existing provider array - newProviders = ts.factory.createArrayLiteralExpression([ - ...providers.elements, - provideHttpClient, - provideHttpClientTesting, + const provideHttpClientTesting = createCallExpression(PROVIDE_HTTP_CLIENT_TESTING); + + // Adding the new providers + const providers = getProvidersFromLiteralExpr(configureTestingModuleArgs); + + let newProviders: ts.ArrayLiteralExpression; + if (!providers) { + // No existing providers, we add a property to the literal + newProviders = ts.factory.createArrayLiteralExpression([ + provideHttpClient, + provideHttpClientTesting, + ]); + } else { + // We add the provider to the existing provider array + newProviders = ts.factory.createArrayLiteralExpression([ + ...providers.elements, + provideHttpClient, + provideHttpClientTesting, + ]); + } + + // Replacing the existing configuration with the new one (with the new imports and providers) + const newTestingModuleArgs = ts.factory.createObjectLiteralExpression([ + ...configureTestingModuleArgs.properties.filter((p) => p.getText() === 'imports'), + ts.factory.createPropertyAssignment('imports', newImports), + ts.factory.createPropertyAssignment('providers', newProviders), ]); + changeTracker.replaceNode(configureTestingModuleArgs, newTestingModuleArgs); } - - // Replacing the existing decorator with the new one (with the new imports and providers) - const newTestingModuleArgs = ts.factory.createObjectLiteralExpression([ - ...configureTestingModuleArgs.properties.filter((p) => p.getText() === 'imports'), - ts.factory.createPropertyAssignment('imports', newImports), - ts.factory.createPropertyAssignment('providers', newProviders), - ]); - changeTracker.replaceNode(configureTestingModuleArgs, newTestingModuleArgs); } function getImportsProp(literal: ts.ObjectLiteralExpression) { @@ -387,3 +441,21 @@ function createCallExpression(functionName: string, args: ts.Expression[] = []) args, ); } + +/** + * Gets the index after the last import in a file. Can be used to insert new code into the file. + * @param sourceFile File in which to search for imports. + */ +function getLastImportEnd(sourceFile: ts.SourceFile): number { + let index = 0; + + for (const statement of sourceFile.statements) { + if (ts.isImportDeclaration(statement)) { + index = Math.max(index, statement.getEnd()); + } else { + break; + } + } + + return index; +} diff --git a/packages/core/schematics/test/http_providers_spec.ts b/packages/core/schematics/test/http_providers_spec.ts index d075ec139d0bc9..efb7c131f1ddfe 100644 --- a/packages/core/schematics/test/http_providers_spec.ts +++ b/packages/core/schematics/test/http_providers_spec.ts @@ -241,7 +241,7 @@ describe('Http providers migration', () => { ); }); - it('should handle a migration for acomponent ', async () => { + it('should handle a migration for a component', async () => { writeFile( '/index.ts', ` @@ -264,6 +264,32 @@ describe('Http providers migration', () => { expect(content).toContain(`provideHttpClient(withInterceptorsFromDi(), withJsonpSupport())`); }); + it('should handle a migration of HttpClientModule in a test', async () => { + writeFile( + '/index.ts', + ` + import { HttpClientModule } from '@angular/common/http'; + + describe('MyComponent', () => { + beforeEach(() => + TestBed.configureTestingModule({ + imports: [HttpClientModule] + }) + ); + }); + `, + ); + + await runMigration(); + + const content = tree.readContent('/index.ts'); + expect(content).not.toContain(`'@angular/common/http/testing'`); + expect(content).toContain(`'@angular/common/http'`); + expect(content).toMatch(/import.*provideHttpClient.*withInterceptorsFromDi.*from/); + expect(content).not.toContain(`HttpClientModule`); + expect(content).toContain(`provideHttpClient(withInterceptorsFromDi())`); + }); + it('should not migrate HttpClientModule from another package', async () => { writeFile( '/index.ts', @@ -318,7 +344,10 @@ describe('Http providers migration', () => { await runMigration(); const content = tree.readContent('/index.ts'); - expect(content).toContain(`@angular/common/http/testing`); + console.log(content); + expect(content).toContain(`'@angular/common/http/testing'`); + expect(content).toContain(`'@angular/common/http'`); + expect(content).toMatch(/import.*provideHttpClient.*withInterceptorsFromDi.*from/); expect(content).not.toContain(`HttpClientTestingModule`); expect(content).toMatch(/import.*provideHttpClientTesting/); expect(content).toContain( @@ -347,7 +376,7 @@ describe('Http providers migration', () => { expect(content).not.toContain('provideHttpClientTesting'); }); - it('shouldmigrate NgModule + TestBed.configureTestingModule in the same file', async () => { + it('should migrate NgModule + TestBed.configureTestingModule in the same file', async () => { writeFile( '/index.ts', ` diff --git a/packages/core/schematics/utils/change_tracker.ts b/packages/core/schematics/utils/change_tracker.ts index e22f4f21fa7ba9..c6e860aa53aeae 100644 --- a/packages/core/schematics/utils/change_tracker.ts +++ b/packages/core/schematics/utils/change_tracker.ts @@ -110,6 +110,8 @@ export class ChangeTracker { * @param sourceFile File to which to add the import. * @param symbolName Symbol being imported. * @param moduleName Module from which the symbol is imported. + * @param alias Alias to use for the import. + * @param keepSymbolName Whether to keep the symbol name in the import. */ addImport( sourceFile: ts.SourceFile,