Skip to content

Commit

Permalink
fix(material/schematics): migrate more cases in the theming API schem…
Browse files Browse the repository at this point in the history
…atic (#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.

(cherry picked from commit 94bb518)
  • Loading branch information
crisbeto authored and annieyw committed May 3, 2021
1 parent efc5952 commit 8b6fdac
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 21 deletions.
4 changes: 2 additions & 2 deletions src/material/schematics/ng-generate/theming-api/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)',
Expand Down
41 changes: 37 additions & 4 deletions src/material/schematics/ng-generate/theming-api/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', [
Expand Down Expand Up @@ -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';`,
Expand All @@ -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;`,
`}`,
Expand Down Expand Up @@ -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;`,
`}`,
Expand Down Expand Up @@ -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();`,
``,
]);
});

});
2 changes: 1 addition & 1 deletion src/material/schematics/ng-generate/theming-api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
32 changes: 18 additions & 14 deletions src/material/schematics/ng-generate/theming-api/migration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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. */
Expand Down

0 comments on commit 8b6fdac

Please sign in to comment.