fix(core): Do not migrate HttpClientModule imports on components.

`provideHttpClient()` returns a `EnvironmentProvider` which is not compatible with component providers.
angular · May 24, 2024 · d0848fe · d0848fe
1 parent 8250238
commit d0848fe
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 2 deletions.
diff --git a/packages/core/schematics/migrations/http-providers/utils.ts b/packages/core/schematics/migrations/http-providers/utils.ts
@@ -69,6 +69,7 @@ export function migrateFile(
           commonHttpTestingIdentifiers,
           addedImports,
           changeTracker,
+          sourceFile,
         );
       });
     }
@@ -146,6 +147,7 @@ function migrateDecorator(
   commonHttpTestingIdentifiers: Set<string>,
   addedImports: Map<string, Set<string>>,
   changeTracker: ChangeTracker,
+  sourceFile: ts.SourceFile,
 ) {
   // Only @NgModule and @Component support `imports`.
   // Also skip decorators with no arguments.
@@ -177,6 +179,22 @@ function migrateDecorator(
     return;
   }
 
+  // HttpClient imported in component is actually a mistake
+  // Schematics will be no-op but add a TODO
+  const isComponent = decorator.name === 'Component';
+  if (isComponent && importedModules.client) {
+    const httpClientModuleIdentifier = importedModules.client;
+    const commentText = `\n// TODO: HttpClientModule should never be imported into a component directly.\n`;
+    ts.addSyntheticLeadingComment(
+      httpClientModuleIdentifier,
+      ts.SyntaxKind.SingleLineCommentTrivia,
+      commentText,
+      true,
+    );
+    changeTracker.insertText(sourceFile, httpClientModuleIdentifier.getStart(), commentText);
+    return;
+  }
+
   const addedProviders = new Set<ts.CallExpression>();
 
   // Handle the different imported Http modules

diff --git a/packages/core/schematics/test/http_providers_spec.ts b/packages/core/schematics/test/http_providers_spec.ts
@@ -264,8 +264,11 @@ describe('Http providers migration', () => {
 
     const content = tree.readContent('/index.ts');
     expect(content).toContain(`@angular/common/http`);
-    expect(content).not.toContain(`HttpClientModule`);
-    expect(content).toContain(`provideHttpClient(withInterceptorsFromDi(), withJsonpSupport())`);
+    expect(content).toContain(`HttpClientModule`);
+    expect(content).not.toContain(
+      `provideHttpClient(withInterceptorsFromDi(), withJsonpSupport())`,
+    );
+    expect(content).toContain('// TODO: HttpClientModule should never be imported');
     expect(content).toContain(`template: ''`);
   });