Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug(material/schematics): v12 theming API migration should not rename variables appended with extra characters #24013

Closed
1 task
Den-dp opened this issue Nov 25, 2021 · 1 comment · Fixed by #24021
Assignees
Labels
area: material/core P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@Den-dp
Copy link

Den-dp commented Nov 25, 2021

Is this a regression?

  • Yes, this behavior used to work in the previous version

The previous version in which this bug was not present was

No response

Description

after running ng update @angular/material I end up with invalid scss:

- $swift-ease-in-duration: 300ms !default;
- $swift-ease-in-timing-function: cubic-bezier(0.55, 0, 0.55, 0.2) !default;
+ all 300ms cubic-bezier(0.55, 0, 0.55, 0.2)-duration: 300ms !default;
+ all 300ms cubic-bezier(0.55, 0, 0.55, 0.2)-timing-function: cubic-bezier(0.55, 0, 0.55, 0.2) !default;

probably related to

'swift-ease-in': 'all 300ms cubic-bezier(0.55, 0, 0.55, 0.2)',

and
/**
* Replaces variables that have been removed with their values.
* @param content Content of the file to be migrated.
* @param variables Mapping between variable names and their values.
*/
function replaceRemovedVariables(content: string, variables: Record<string, string>): string {
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');
content = content.replace(regex, variables[variableName]);
});
return content;
}

/** Escapes special regex characters in a string. */
function escapeRegExp(str: string): string {
return str.replace(/([.*+?^=!:${}()|[\]\/\\])/g, '\\$1');
}

Reproduction

I was able to reproduce it by running yarn test material/schematics in this repo since there is a test for it

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();`,
]);
});

Expected Behavior

should not rename variables appended with extra characters

Actual Behavior

it renames it in an incorrect way

Environment

  • CDK/Material: 13 and I think 12
@Den-dp Den-dp added the needs triage This issue needs to be triaged by the team label Nov 25, 2021
@crisbeto crisbeto self-assigned this Nov 27, 2021
@crisbeto crisbeto added area: material/core has pr P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent and removed needs triage This issue needs to be triaged by the team labels Nov 27, 2021
crisbeto added a commit to crisbeto/material2 that referenced this issue Nov 27, 2021
…e names of other variables in theming API migration

Fixes that the theming API migration would replace the `$swift-ease-in` inside a variable like `$swift-ease-in-duration`, because the regex would partially match the name.

Fixes angular#24013.
amysorto pushed a commit that referenced this issue Dec 14, 2021
…e names of other variables in theming API migration (#24021)

Fixes that the theming API migration would replace the `$swift-ease-in` inside a variable like `$swift-ease-in-duration`, because the regex would partially match the name.

Fixes #24013.
amysorto pushed a commit that referenced this issue Dec 14, 2021
…e names of other variables in theming API migration (#24021)

Fixes that the theming API migration would replace the `$swift-ease-in` inside a variable like `$swift-ease-in-duration`, because the regex would partially match the name.

Fixes #24013.

(cherry picked from commit 7468588)
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 14, 2022
forsti0506 pushed a commit to forsti0506/components that referenced this issue Apr 3, 2022
…e names of other variables in theming API migration (angular#24021)

Fixes that the theming API migration would replace the `$swift-ease-in` inside a variable like `$swift-ease-in-duration`, because the regex would partially match the name.

Fixes angular#24013.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: material/core P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
2 participants