diff --git a/src/material/schematics/ng-update/migrations/theming-api-v12/config.ts b/src/material/schematics/ng-update/migrations/theming-api-v12/config.ts index 01d364e43fbc..655982261bef 100644 --- a/src/material/schematics/ng-update/migrations/theming-api-v12/config.ts +++ b/src/material/schematics/ng-update/migrations/theming-api-v12/config.ts @@ -138,6 +138,9 @@ export const removedMaterialVariables: Record = { 'mat-expansion-panel-header-expanded-minimum-height': '48px', 'mat-expansion-panel-header-expanded-maximum-height': '64px', 'mat-expansion-panel-header-transition': '225ms cubic-bezier(0.4, 0, 0.2, 1)', + 'mat-menu-side-padding': '16px', + 'menu-menu-item-height': '48px', + 'menu-menu-icon-margin': '16px', 'mat-paginator-height': '56px', 'mat-paginator-minimum-height': '40px', 'mat-paginator-maximum-height': '56px', diff --git a/src/material/schematics/ng-update/migrations/theming-api-v12/migration.ts b/src/material/schematics/ng-update/migrations/theming-api-v12/migration.ts index c72ce8a7ce5b..a66116fafb15 100644 --- a/src/material/schematics/ng-update/migrations/theming-api-v12/migration.ts +++ b/src/material/schematics/ng-update/migrations/theming-api-v12/migration.ts @@ -56,9 +56,9 @@ export function migrateFileContent(content: string, // 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 = migrateCdkSymbols(content, newCdkImportPath, cdkResults); content = migrateMaterialSymbols( content, newMaterialImportPath, materialResults, extraMaterialSymbols); - content = migrateCdkSymbols(content, newCdkImportPath, cdkResults); content = replaceRemovedVariables(content, removedMaterialVariables); // We can assume that the migration has taken care of any Components symbols that were @@ -142,7 +142,7 @@ function migrateMaterialSymbols(content: string, importPath: string, if (content !== initialContent) { // Add an import to the new API only if any of the APIs were being used. - content = insertUseStatement(content, importPath, detectedImports.imports, namespace); + content = insertUseStatement(content, importPath, namespace); } return content; @@ -165,7 +165,7 @@ function migrateCdkSymbols(content: string, importPath: string, // Previously the CDK symbols were exposed through `material/theming`, but now we have a // dedicated entrypoint for the CDK. Only add an import for it if any of the symbols are used. if (content !== initialContent) { - content = insertUseStatement(content, importPath, detectedImports.imports, namespace); + content = insertUseStatement(content, importPath, namespace); } return content; @@ -186,10 +186,8 @@ function renameSymbols(content: string, getKeyPattern: (namespace: string|null, key: string) => RegExp, formatValue: (key: string) => string): string { // The null at the end is so that we make one last pass to cover non-namespaced symbols. - [...namespaces.slice().sort(sortLengthDescending), null].forEach(namespace => { - // Migrate the longest keys first so that our regex-based replacements don't accidentally - // capture keys that contain other keys. E.g. `$mat-blue` is contained within `$mat-blue-grey`. - Object.keys(mapping).sort(sortLengthDescending).forEach(key => { + [...namespaces.slice(), null].forEach(namespace => { + Object.keys(mapping).forEach(key => { const pattern = getKeyPattern(namespace, key); // Sanity check since non-global regexes will only replace the first match. @@ -205,26 +203,24 @@ function renameSymbols(content: string, } /** Inserts an `@use` statement in a string. */ -function insertUseStatement(content: string, importPath: string, importsToIgnore: string[], - namespace: string): string { +function insertUseStatement(content: string, importPath: string, namespace: string): string { // If the content already has the `@use` import, we don't need to add anything. - const alreadyImportedPattern = new RegExp(`@use +['"]${importPath}['"]`, 'g'); - if (alreadyImportedPattern.test(content)) { + if (new RegExp(`@use +['"]${importPath}['"]`, 'g').test(content)) { return content; } - // We want to find the first import that isn't in the list of ignored imports or find nothing, - // because the imports being replaced might be the only ones in the file and they can be further - // down. An easy way to do this is to replace the imports with a random character and run - // `indexOf` on the result. This isn't the most efficient way of doing it, but it's more compact - // and it allows us to easily deal with things like comment nodes. - const contentToSearch = importsToIgnore.reduce((accumulator, current) => - accumulator.replace(current, '◬'.repeat(current.length)), content); - - // Sass has a limitation that all `@use` declarations have to come before `@import` so we have - // to find the first import and insert before it. Technically we can get away with always - // inserting at 0, but the file may start with something like a license header. - const newImportIndex = Math.max(0, contentToSearch.indexOf('@import ')); + // Sass will throw an error if an `@use` statement comes after another statement. The safest way + // to ensure that we conform to that requirement is by always inserting our imports at the top + // of the file. Detecting where the user's content starts is tricky, because there are many + // different kinds of syntax we'd have to account for. One approach is to find the first `@import` + // and insert before it, but the problem is that Sass allows `@import` to be placed anywhere. + let newImportIndex = 0; + + // One special case is if the file starts with a license header which we want to preserve on top. + if (content.trim().startsWith('/*')) { + const commentEndIndex = content.indexOf('*/', content.indexOf('/*')); + newImportIndex = content.indexOf('\n', commentEndIndex) + 1; + } return content.slice(0, newImportIndex) + `@use '${importPath}' as ${namespace};\n` + content.slice(newImportIndex); @@ -247,7 +243,8 @@ function getMixinValueFormatter(namespace: string): (name: string) => string { /** Formats a migration key as a Sass function invocation. */ function functionKeyFormatter(namespace: string|null, name: string): RegExp { - return new RegExp(escapeRegExp(`${namespace ? namespace + '.' : ''}${name}(`), 'g'); + const functionName = escapeRegExp(`${namespace ? namespace + '.' : ''}${name}(`); + return new RegExp(`(? string /** Formats a migration key as a Sass variable. */ function variableKeyFormatter(namespace: string|null, name: string): RegExp { - return new RegExp(escapeRegExp(`${namespace ? namespace + '.' : ''}$${name}`), 'g'); + const variableName = escapeRegExp(`${namespace ? namespace + '.' : ''}$${name}`); + return new RegExp(`${variableName}(?![-_a-zA-Z0-9])`, 'g'); } /** Returns a function that can be used to format a Sass variable replacement. */ @@ -270,11 +268,6 @@ function escapeRegExp(str: string): string { return str.replace(/([.*+?^=!:${}()|[\]\/\\])/g, '\\$1'); } -/** Used with `Array.prototype.sort` to order strings in descending length. */ -function sortLengthDescending(a: string, b: string) { - return b.length - a.length; -} - /** Removes all strings from another string. */ function removeStrings(content: string, toRemove: string[]): string { return toRemove @@ -325,7 +318,7 @@ function extractNamespaceFromUseStatement(fullImport: string): string { * @param variables Mapping between variable names and their values. */ function replaceRemovedVariables(content: string, variables: Record): string { - Object.keys(variables).sort(sortLengthDescending).forEach(variableName => { + Object.keys(variables).forEach(variableName => { // Note that the pattern uses a negative lookahead to exclude // variable assignments, because they can't be migrated. const regex = new RegExp(`\\$${escapeRegExp(variableName)}(?!\\s+:|:)`, 'g'); diff --git a/src/material/schematics/ng-update/test-cases/v12/misc/theming-api-v12.spec.ts b/src/material/schematics/ng-update/test-cases/v12/misc/theming-api-v12.spec.ts index 32cfc9abc820..af7cd5913cf9 100644 --- a/src/material/schematics/ng-update/test-cases/v12/misc/theming-api-v12.spec.ts +++ b/src/material/schematics/ng-update/test-cases/v12/misc/theming-api-v12.spec.ts @@ -272,8 +272,8 @@ describe('v12 theming API migration', () => { await runMigration(); expect(splitFile(THEME_PATH)).toEqual([ - `@use './foo'`, `@use '~@angular/material' as mat;`, + `@use './foo'`, `@import './bar'`, `@include mat.core();`, ]); @@ -424,8 +424,8 @@ describe('v12 theming API migration', () => { await runMigration(); expect(splitFile(THEME_PATH)).toEqual([ - `@use '~@angular/cdk' as cdk;`, `@use '~@angular/material' as mat;`, + `@use '~@angular/cdk' as cdk;`, `@include cdk.overlay();`, @@ -754,4 +754,54 @@ describe('v12 theming API migration', () => { `$another: mat.$pink-palette;`, ]); }); + + it('should insert @use before other code when only Angular imports are first', async () => { + writeLines(THEME_PATH, [ + `@import '~@angular/material/theming';`, + `$something: 123;`, + `@include mat-core();`, + `@import 'some/other/file';`, + ]); + + await runMigration(); + + expect(splitFile(THEME_PATH)).toEqual([ + `@use '~@angular/material' as mat;`, + `$something: 123;`, + `@include mat.core();`, + `@import 'some/other/file';`, + ]); + }); + + it('should not rename variables appended with extra characters', async () => { + writeLines(THEME_PATH, [ + `@import '~@angular/material/theming';`, + `$mat-light-theme-background-override: 123;`, + `@include mat-core();`, + ]); + + await runMigration(); + + expect(splitFile(THEME_PATH)).toEqual([ + `@use '~@angular/material' as mat;`, + `$mat-light-theme-background-override: 123;`, + `@include mat.core();`, + ]); + }); + + it('should not rename functions prepended with extra characters', async () => { + writeLines(THEME_PATH, [ + `@function gmat-palette() {`, + ` @return white;`, + `}`, + ]); + + await runMigration(); + + expect(splitFile(THEME_PATH)).toEqual([ + `@function gmat-palette() {`, + ` @return white;`, + `}`, + ]); + }); });