fix(material/schematics): migrate more cases in the theming API schematic (#22604)

Includes the following improvements to the `themingApi` schematic:

1. No longer runs against files in the `node_modules`. This was happening by accident.
2. Remove the quotes around the values of `$mat-small` and `$mat-xsmall` since they were causing Sass syntax errors.
3. Migrates Material/CDK symbols, even if there are no imports for the old theming bundle. This allows us to handle symbols that were imported transitively.
4. Reverts the logic from #22438 that doesn't drop imports if there are no Material/CDK symbol usages within the file. I think that we want to do this after all, because the old import could slow down build times and we can be relatively certain that any usages have been migrated.

Fixes #22599.

Author: crisbeto

src/material/schematics/ng-generate/theming-api/config.ts

@@ -103,8 +103,8 @@ export const cdkMixins: Record<string, string> = {
 export const removedMaterialVariables: Record<string, string> = {
   // Note: there's also a usage of a variable called `$pi`, but the name is short enough that
   // it matches things like `$mat-pink`. Don't migrate it since it's unlikely to be used.
-  'mat-xsmall': `'max-width: 599px'`,
-  'mat-small': `'max-width: 959px'`,
+  'mat-xsmall': 'max-width: 599px',
+  'mat-small': 'max-width: 959px',
   'mat-toggle-padding': '8px',
   'mat-toggle-size': '20px',
   'mat-linear-out-slow-in-timing-function': 'cubic-bezier(0, 0, 0.2, 0.1)',

src/material/schematics/ng-generate/theming-api/index.spec.ts

@@ -195,6 +195,23 @@ describe('Material theming API schematic', () => {
     ]);
   });
 
+  it('should migrate files that import the Material APIs transitively', async () => {
+    const app = await createTestApp(runner);
+    app.create('/theme.scss', [
+      `@import 're-exports-material-symbols';`,
+      `@include mat-core();`,
+      `@include mat-button-theme();`,
+    ].join('\n'));
+
+    const tree = await runner.runSchematicAsync('theming-api', options, app).toPromise();
+    expect(getFileContent(tree, '/theme.scss').split('\n')).toEqual([
+      `@use '~@angular/material' as mat;`,
+      `@import 're-exports-material-symbols';`,
+      `@include mat.core();`,
+      `@include mat.button-theme();`,
+    ]);
+  });
+
   it('should allow an arbitrary number of spaces after @include and @import', async () => {
     const app = await createTestApp(runner);
     app.create('/theme.scss', [
@@ -485,7 +502,7 @@ describe('Material theming API schematic', () => {
     ]);
   });
 
-  it('should not change files if they have an import, but do not use any symbols', async () => {
+  it('should drop the old import path even if the file is not using any symbols', async () => {
     const app = await createTestApp(runner);
     app.create('/theme.scss', [
       `@import '~@angular/material/theming';`,
@@ -497,8 +514,6 @@ describe('Material theming API schematic', () => {
 
     const tree = await runner.runSchematicAsync('theming-api', options, app).toPromise();
     expect(getFileContent(tree, '/theme.scss').split('\n')).toEqual([
-      `@import '~@angular/material/theming';`,
-      ``,
       `.my-dialog {`,
         `color: red;`,
       `}`,
@@ -537,7 +552,7 @@ describe('Material theming API schematic', () => {
         `transition: all 400ms cubic-bezier(0.25, 0.8, 0.25, 1);`,
       `}`,
       ``,
-      `@media ('max-width: 959px') {`,
+      `@media (max-width: 959px) {`,
         `.my-button-toggle {`,
           `height: 24px;`,
         `}`,
@@ -571,4 +586,22 @@ describe('Material theming API schematic', () => {
     ]);
   });
 
+  it('should not migrate files in the node_modules', async () => {
+    const app = await createTestApp(runner);
+    app.create('/node_modules/theme.scss', [
+      `@import '~@angular/material/theming';`,
+      ``,
+      `@include mat-button-toggle-theme();`,
+      ``,
+    ].join('\n'));
+
+    const tree = await runner.runSchematicAsync('theming-api', options, app).toPromise();
+    expect(getFileContent(tree, '/node_modules/theme.scss').split('\n')).toEqual([
+      `@import '~@angular/material/theming';`,
+      ``,
+      `@include mat-button-toggle-theme();`,
+      ``,
+    ]);
+  });
+
 });

src/material/schematics/ng-generate/theming-api/index.ts

@@ -14,7 +14,7 @@ import {migrateFileContent} from './migration';
 export default function(_options: Schema): Rule {
   return (tree: Tree) => {
     tree.visit((path, entry) => {
-      if (extname(path) === '.scss') {
+      if (extname(path) === '.scss' && path.indexOf('node_modules') === -1) {
         const content = entry?.content.toString();
         const migratedContent = content ? migrateFileContent(content,
           '~@angular/material/', '~@angular/cdk/', '~@angular/material', '~@angular/cdk') : content;

src/material/schematics/ng-generate/theming-api/migration.ts

@@ -37,19 +37,21 @@ export function migrateFileContent(content: string,
   const materialResults = detectImports(content, oldMaterialPrefix);
   const cdkResults = detectImports(content, oldCdkPrefix);
 
-  // If there are no imports, we don't need to go further.
-  if (materialResults.imports.length > 0 || cdkResults.imports.length > 0) {
-    const initialContent = content;
-    content = migrateMaterialSymbols(content, newMaterialImportPath, materialResults.namespaces);
-    content = migrateCdkSymbols(content, newCdkImportPath, cdkResults.namespaces);
-
-    // Only drop the imports if any of the symbols were used within the file.
-    if (content !== initialContent) {
-      content = removeStrings(content, materialResults.imports);
-      content = removeStrings(content, cdkResults.imports);
-      content = content.replace(/^\s+/, '');
-      content = replaceRemovedVariables(content, removedMaterialVariables);
-    }
+  // Try to migrate the symbols even if there are no imports. This is used
+  // to cover the case where the Components symbols were used transitively.
+  content = migrateMaterialSymbols(content, newMaterialImportPath, materialResults.namespaces);
+  content = migrateCdkSymbols(content, newCdkImportPath, cdkResults.namespaces);
+  content = replaceRemovedVariables(content, removedMaterialVariables);
+
+  // We can assume that the migration has taken care of any Components symbols that were
+  // imported transitively so we can always drop the old imports. We also assume that imports
+  // to the new entry points have been added already.
+  if (materialResults.imports.length) {
+    content = removeStrings(content, materialResults.imports);
+  }
+
+  if (cdkResults.imports.length) {
+    content = removeStrings(content, cdkResults.imports);
   }
 
   return content;
@@ -227,7 +229,9 @@ function sortLengthDescending(a: string, b: string) {
 
 /** Removes all strings from another string. */
 function removeStrings(content: string, toRemove: string[]): string {
-  return toRemove.reduce((accumulator, current) => accumulator.replace(current, ''), content);
+  return toRemove
+    .reduce((accumulator, current) => accumulator.replace(current, ''), content)
+    .replace(/^\s+/, '');
 }
 
 /** Parses out the namespace from a Sass `@use` statement. */