From ca7515593cd2ec53f2bc09528285dcdda6b7cc77 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Thu, 10 Nov 2022 15:27:02 +0100 Subject: [PATCH] fix(material/schematics): handle input level in typography migration Expands the logic in the MDC theming migration to be able to discern both keys and values of named Sass function parameters. The new logic is used to correctly migrate users away from the `input` typography levels which doesn't have an equivalent in the new system. --- .../components/typography-config.spec.ts | 110 ++++++++++++- .../typography-hierarchy/constants.ts | 3 + .../mdc-migration/rules/theming-styles.ts | 152 ++++++++++++++++-- 3 files changed, 253 insertions(+), 12 deletions(-) diff --git a/src/material/schematics/ng-generate/mdc-migration/rules/components/typography-config.spec.ts b/src/material/schematics/ng-generate/mdc-migration/rules/components/typography-config.spec.ts index 95a9a9274be6..29b39870582b 100644 --- a/src/material/schematics/ng-generate/mdc-migration/rules/components/typography-config.spec.ts +++ b/src/material/schematics/ng-generate/mdc-migration/rules/components/typography-config.spec.ts @@ -103,7 +103,6 @@ describe('typography migrations', () => { $body-1: $body-1, $caption: $caption, $button: $button, - $input: $input, ), )); `, @@ -130,7 +129,6 @@ describe('typography migrations', () => { $body-2: $body-1, $caption: $caption, $button: $button, - $input: $input, ), )); `, @@ -196,4 +194,112 @@ describe('typography migrations', () => { `, ); }); + + it('should migrate a typography config with a mixture of positional and named arguments', async () => { + await runMigrationTest( + THEME_FILE, + ` + @use '@angular/material' as mat; + + $sample-project-theme: mat.define-light-theme(( + color: ( + primary: $sample-project-primary, + accent: $sample-project-accent, + warn: $sample-project-warn, + ), + typography: mat.define-legacy-typography-config($font-family, $display-4: $custom-display-4, $display-3), + )); + `, + ` + @use '@angular/material' as mat; + + $sample-project-theme: mat.define-light-theme(( + color: ( + primary: $sample-project-primary, + accent: $sample-project-accent, + warn: $sample-project-warn, + ), + typography: mat.define-typography-config($font-family, $headline-1: $custom-display-4, $display-3), + )); + `, + ); + }); + + it('should replace the `input` level with `body-1` if there is no `body-1` in the config', async () => { + await runMigrationTest( + THEME_FILE, + ` + @use '@angular/material' as mat; + + $sample-project-theme: mat.define-light-theme(( + color: ( + primary: $sample-project-primary, + accent: $sample-project-accent, + warn: $sample-project-warn, + ), + typography: mat.define-legacy-typography-config( + $display-4: $display-4, + $display-3: $display-3, + $input: $input, + ), + )); + `, + ` + @use '@angular/material' as mat; + + $sample-project-theme: mat.define-light-theme(( + color: ( + primary: $sample-project-primary, + accent: $sample-project-accent, + warn: $sample-project-warn, + ), + typography: mat.define-typography-config( + $headline-1: $display-4, + $headline-2: $display-3, + $body-1: $input, + ), + )); + `, + ); + }); + + it('should comment out the `input` level if a `body-1` already exists', async () => { + await runMigrationTest( + THEME_FILE, + ` + @use '@angular/material' as mat; + + $sample-project-theme: mat.define-light-theme(( + color: ( + primary: $sample-project-primary, + accent: $sample-project-accent, + warn: $sample-project-warn, + ), + typography: mat.define-legacy-typography-config( + $display-4: $display-4, + $display-3: $display-3, + $input: $input, + $subheading-1: $subheading-1, + ), + )); + `, + ` + @use '@angular/material' as mat; + + $sample-project-theme: mat.define-light-theme(( + color: ( + primary: $sample-project-primary, + accent: $sample-project-accent, + warn: $sample-project-warn, + ), + typography: mat.define-typography-config( + $headline-1: $display-4, + $headline-2: $display-3, + /* TODO(mdc-migration): No longer supported. Use \`body-1\` instead. $input: $input, */ + $body-1: $subheading-1, + ), + )); + `, + ); + }); }); diff --git a/src/material/schematics/ng-generate/mdc-migration/rules/components/typography-hierarchy/constants.ts b/src/material/schematics/ng-generate/mdc-migration/rules/components/typography-hierarchy/constants.ts index e2a2ed61a8c7..affc67ce7755 100644 --- a/src/material/schematics/ng-generate/mdc-migration/rules/components/typography-hierarchy/constants.ts +++ b/src/material/schematics/ng-generate/mdc-migration/rules/components/typography-hierarchy/constants.ts @@ -29,3 +29,6 @@ export const RENAMED_TYPOGRAPHY_LEVELS = new Map(mappings); export const RENAMED_TYPOGRAPHY_CLASSES = new Map( mappings.map(m => ['mat-' + m[0], 'mat-' + m[1]]), ); + +/** Typography levels that have been combined into other levels with no replacement. */ +export const COMBINED_TYPOGRAPHY_LEVELS = new Map([['input', 'body-1']]); diff --git a/src/material/schematics/ng-generate/mdc-migration/rules/theming-styles.ts b/src/material/schematics/ng-generate/mdc-migration/rules/theming-styles.ts index 83c4303c6ffa..fba06529f0b1 100644 --- a/src/material/schematics/ng-generate/mdc-migration/rules/theming-styles.ts +++ b/src/material/schematics/ng-generate/mdc-migration/rules/theming-styles.ts @@ -11,7 +11,10 @@ import {SchematicContext} from '@angular-devkit/schematics'; import * as postcss from 'postcss'; import * as scss from 'postcss-scss'; import {ComponentMigrator, MIGRATORS, PERMANENT_MIGRATORS} from '.'; -import {RENAMED_TYPOGRAPHY_LEVELS} from './components/typography-hierarchy/constants'; +import { + COMBINED_TYPOGRAPHY_LEVELS, + RENAMED_TYPOGRAPHY_LEVELS, +} from './components/typography-hierarchy/constants'; import {StyleMigrator} from './style-migrator'; const COMPONENTS_MIXIN_NAME = /\.([^(;]*)/; @@ -259,23 +262,48 @@ function migrateTypographyConfigs(content: string, namespace: string): string { const replacements: {start: number; end: number; text: string}[] = []; calls.forEach(({name, args}) => { - const argContent = content.slice(args.start, args.end); - replacements.push({start: name.start, end: name.end, text: newFunctionName}); + const parameters = extractNamedParameters(content, args); + const addedParameters = new Set(); RENAMED_TYPOGRAPHY_LEVELS.forEach((newName, oldName) => { - const pattern = new RegExp(`\\$(${oldName}) *:`, 'g'); - let match: RegExpExecArray | null; + const correspondingParam = parameters.get(oldName); - // Technically each argument can only match once, but keep going just in case. - while ((match = pattern.exec(argContent))) { - const start = args.start + match.index + 1; + if (correspondingParam) { + addedParameters.add(newName); replacements.push({ - start, - end: start + match[1].length, + start: correspondingParam.key.start + 1, // + 1 to skip over the $ in the parameter name. + end: correspondingParam.key.end, text: newName, }); } }); + + COMBINED_TYPOGRAPHY_LEVELS.forEach((newName, oldName) => { + const correspondingParam = parameters.get(oldName); + + if (correspondingParam) { + if (addedParameters.has(newName)) { + const fullContent = content.slice( + correspondingParam.key.start, + correspondingParam.value.fullEnd, + ); + replacements.push({ + start: correspondingParam.key.start, + end: correspondingParam.value.fullEnd, + text: `/* TODO(mdc-migration): No longer supported. Use \`${newName}\` instead. ${fullContent} */`, + }); + } else { + addedParameters.add(newName); + replacements.push({ + start: correspondingParam.key.start + 1, // + 1 to skip over the $ in the parameter name. + end: correspondingParam.key.end, + text: newName, + }); + } + } + }); + + replacements.push({start: name.start, end: name.end, text: newFunctionName}); }); replacements @@ -330,3 +358,107 @@ function extractFunctionCalls(name: string, content: string) { return results; } + +/** Extracts all of the named parameters and their values from a string. */ +function extractNamedParameters(content: string, argsRange: {start: number; end: number}) { + let escapeCount = 0; + + const args = content + .slice(argsRange.start, argsRange.end) + // The top-level function parameters can contain function calls with named parameters of their + // own (e.g. `$display-4: mat.define-typography-level($font-family: $foo))` which we don't want to + // extract. Escape everything between parentheses to make it easier to parse out the value later + // on. Note that we escape with an equal-length string so that the string indexes remain the same. + .replace(/\(.*\)/g, current => ++escapeCount + '◬'.repeat(current.length - 1)); + + let colonIndex = args.indexOf(':'); + + const params = new Map< + string, + {key: {start: number; end: number}; value: {start: number; end: number; fullEnd: number}} + >(); + + while (colonIndex > -1) { + const keyRange = extractKeyRange(args, colonIndex); + const valueRange = extractValueRange(args, colonIndex); + + if (keyRange && valueRange) { + // + 1 to exclude the $ in the key name. + params.set(args.slice(keyRange.start + 1, keyRange.end), { + // Add the argument start offset since the indexes are relative to the argument string. + key: {start: keyRange.start + argsRange.start, end: keyRange.end + argsRange.start}, + value: { + start: valueRange.start + argsRange.start, + end: valueRange.end + argsRange.start, + fullEnd: valueRange.fullEnd + argsRange.start, + }, + }); + } + + colonIndex = args.indexOf(':', colonIndex + 1); + } + + return params; +} + +/** + * Extracts the text range that contains the key of a named Sass parameter, including the leading $. + * @param content Text content in which to search. + * @param colonIndex Index of the colon between the key and value. + * Used as a starting point for the search. + */ +function extractKeyRange(content: string, colonIndex: number) { + let index = colonIndex - 1; + let start = -1; + let end = -1; + + while (index > -1) { + const char = content[index]; + if (char !== ' ' && char !== '\n') { + if (end === -1) { + end = index + 1; + } else if (char === '$') { + start = index; + break; + } + } + index--; + } + + return start > -1 && end > -1 ? {start, end} : null; +} + +/** + * Extracts the text range that contains the value of a named Sass parameter. + * @param content Text content in which to search. + * @param colonIndex Index of the colon between the key and value. + * Used as a starting point for the search. + */ +function extractValueRange(content: string, colonIndex: number) { + let index = colonIndex + 1; + let start = -1; + let end = -1; + let fullEnd = -1; // This is the end including any separators (e.g. commas). + + while (index < content.length) { + const char = content[index]; + const isWhitespace = char === ' ' || char === '\n'; + + if (!isWhitespace && start === -1) { + start = index; + } else if (start > -1 && (isWhitespace || char === ',')) { + end = index; + fullEnd = index + 1; + break; + } + + if (start > -1 && index === content.length - 1) { + fullEnd = end = content.length; + break; + } + + index++; + } + + return start > -1 && end > -1 ? {start, end, fullEnd} : null; +}