Skip to content

Commit

Permalink
fix(migrations): preserve existing properties in HttpClientModule mig…
Browse files Browse the repository at this point in the history
…ration (#55777)

The `HttpClientModule` migration was dropping the existing properties other than imports and providers when updating an `@NgModule`, `@Component` or `configureTestingModule`.

PR Close #55777
  • Loading branch information
cexbrayat authored and atscott committed May 14, 2024
1 parent beedb68 commit bb4a401
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 1 deletion.
9 changes: 8 additions & 1 deletion packages/core/schematics/migrations/http-providers/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,10 @@ function migrateDecorator(

// Replacing the existing decorator with the new one (with the new imports and providers)
const newDecoratorArgs = ts.factory.createObjectLiteralExpression([
...metadata.properties.filter((p) => p.getText() === 'imports'),
...metadata.properties.filter(
(property) =>
property.name?.getText() !== 'imports' && property.name?.getText() !== 'providers',
),
ts.factory.createPropertyAssignment('imports', newImports),
ts.factory.createPropertyAssignment('providers', newProviders),
]);
Expand Down Expand Up @@ -459,6 +462,10 @@ function updateTestBedConfiguration(
newProviders: ts.ArrayLiteralExpression,
): ts.ObjectLiteralExpression {
return ts.factory.updateObjectLiteralExpression(configureTestingModuleArgs, [
...configureTestingModuleArgs.properties.filter(
(property) =>
property.name?.getText() !== 'imports' && property.name?.getText() !== 'providers',
),
ts.factory.createPropertyAssignment('imports', newImports),
ts.factory.createPropertyAssignment('providers', newProviders),
]);
Expand Down
8 changes: 8 additions & 0 deletions packages/core/schematics/test/http_providers_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,10 @@ describe('Http providers migration', () => {
import { NgModule } from '@angular/core';
import { HttpClientModule, HttpClientJsonpModule, HttpClientXsrfModule, HttpTransferCacheOptions } from '@angular/common/http';
import { CommonModule } from '@angular/common';
import { AppComponent } from './app.component';
@NgModule({
declarations: [AppComponent],
imports: [
CommonModule,
HttpClientModule,HttpClientJsonpModule,
Expand All @@ -98,6 +100,8 @@ describe('Http providers migration', () => {
expect(content).toContain(
`provideHttpClient(withInterceptorsFromDi(), withJsonpSupport(), withXsrfConfiguration({ cookieName: 'foobar' }))`,
);
expect(content).toContain(`RouterModule.forRoot([])`);
expect(content).toContain(`declarations: [AppComponent]`);
});

it('should replace HttpClientModule with existing providers ', async () => {
Expand Down Expand Up @@ -262,6 +266,7 @@ describe('Http providers migration', () => {
expect(content).toContain(`@angular/common/http`);
expect(content).not.toContain(`HttpClientModule`);
expect(content).toContain(`provideHttpClient(withInterceptorsFromDi(), withJsonpSupport())`);
expect(content).toContain(`template: ''`);
});

it('should handle a migration of HttpClientModule in a test', async () => {
Expand Down Expand Up @@ -334,8 +339,10 @@ describe('Http providers migration', () => {
`
import { TestBed } from '@angular/core/testing';
import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing';
import { AppComponent } from './app.component';
TestBed.configureTestingModule({
declarations: [AppComponent],
imports: [HttpClientTestingModule],
});
`,
Expand All @@ -352,6 +359,7 @@ describe('Http providers migration', () => {
expect(content).toContain(
`provideHttpClient(withInterceptorsFromDi()), provideHttpClientTesting()`,
);
expect(content).toContain(`declarations: [AppComponent]`);
});

it('should not migrate HttpClientTestingModule from outside package', async () => {
Expand Down

0 comments on commit bb4a401

Please sign in to comment.