bug(Material Migration m2-theming-v18): Material 18 Migration Fails - Invalid regular expression

[nwilson-newcura]

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

17.3.10

Description

When attempting to run the ng update @angular/material@18 script to make the step from Angular Material 17 to 18, the script breaks consistently, providing the following output:

$ ng update @angular/material@18.0
Using package manager: npm
Collecting installed dependencies...
Found 80 dependencies.
Fetching dependency metadata from registry...
    Updating package.json with dependency @angular/cdk @ "18.0.6" (was "17.3.10")...
    Updating package.json with dependency @angular/material @ "18.0.6" (was "17.3.10")...
    Updating package.json with dependency @angular/material-moment-adapter @ "18.0.6" (was "17.3.10")...
UPDATE package.json (3953 bytes)
✔ Cleaning node modules directory
✔ Installing packages
** Executing migrations of package '@angular/cdk' **

❯ Updates the Angular CDK to v18.
    Skipping migration for project ems-ui-e2e. Unable to determine 'tsconfig.json' file in workspace config.
    
      ✓  Updated Angular CDK to version 18

  Migration completed (No changes made).

** Executing migrations of package '@angular/material' **

❯ Updates Angular Material to v18.
✖ Migration failed: Invalid regular expression: /*\.define-light-theme\(/: Nothing to repeat
  See "C:\Users\NATHAN~1\AppData\Local\Temp\ng-qBQlVT\angular-errors.log" for further details.

The angular-errors.log provides slightly more details:

[error] SyntaxError: Invalid regular expression: /*\.define-light-theme\(/: Nothing to repeat
    at new RegExp (<anonymous>)
    at migrateFunction (C:\git\emsuite\UI\node_modules\@angular\material\schematics\ng-update\index_bundled.js:750:34)
    at migrateM2ThemingApiUsages (C:\git\emsuite\UI\node_modules\@angular\material\schematics\ng-update\index_bundled.js:723:21)
    at M2ThemingMigration2.postAnalysis (C:\git\emsuite\UI\node_modules\@angular\material\schematics\ng-update\index_bundled.js:844:70)
    at C:\git\emsuite\UI\node_modules\@angular\cdk\schematics\update-tool\index.js:137:35
    at Array.forEach (<anonymous>)
    at UpdateProject.migrate (C:\git\emsuite\UI\node_modules\@angular\cdk\schematics\update-tool\index.js:137:20)
    at runMigrations (C:\git\emsuite\UI\node_modules\@angular\cdk\schematics\ng-update\devkit-migration-rule.js:119:42)
    at C:\git\emsuite\UI\node_modules\@angular\cdk\schematics\ng-update\devkit-migration-rule.js:82:17
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

This has been tested by multiple people on our team with the same results. According to the code here ( https://github.com/angular/components/blob/18.0.x/src/material/schematics/ng-update/migrations/m2-theming-v18/migration.ts ) it is checking for commented out function names, but it is not escaping the comment characters, which is causing the migrateFunction function to fail on line 154, since the /* characters are not being escaped.

Reproduction

StackBlitz link: (As this is a migration script run via ng update, a stackblitz does not make sense
Steps to reproduce:

1. Create a new Angular Material project @ 17.3.10.
2. In the application's project directory, run ng update @angular/core@18 @angular/cli@18 to update your application to Angular v18.
3. Run ng update @angular/material@18.

Expected Behavior

I would expect the script to proceed, and finish migrating things from 17 to 18.

Actual Behavior

It crashes out, with the errors above.

Environment

• Angular: 17.3.10 => 18
• CDK/Material: 17.3.10 => 18
• Browser(s): Attempted in both a git bash window and a basic command prompt
• Operating System (e.g. Windows, macOS, Ubuntu): Windows 11.

[crisbeto]

We discussed it and we don't really have the ability to release an older version like this anymore. What do the @use statements in your file look like? I suspect that it's not parsing something correctly which causes it to throw the error further down.

[nwilson-newcura]

The use statements in our theme file are simply:

@use '@angular/material' as mat;
@use "sass:map";

I understand not being able to go back and fix previous versions - but the issue, I'm pretty sure, is simply that /*\.define-light-theme\(/ is not a valid Regex - it's looking for literally /*.define-light-theme( and failing because the /* are not escaped properly. There needs to be an additional function where it looks at ${oldNamespace} (in line 154) which is guaranteed to have /* in it, since this starts out by commenting out code and then searching for code that was commented out - and sanitizes it for regex by escaping reserved characters.

We personally got past this by skipping the migration and performing the tasks manually, but I wanted to pass this along and hope it helps. We had two themes, a dark and a light, which were declared in the themes file, which the migration could not get past.

[igcherkaev]

I've modified node_modules/@angular/material/schematics/ng-update/index_bundled.js at line 867 to look like this:

    function migrateM2ThemingApiUsages(fileContent) {
      let { content, placeholders } = escapeComments(fileContent);
      const materialNamespaces = getNamespaces("@angular/material", content);
      const experimentalNamespaces = getNamespaces("@angular/material-experimental", content);
      for (let namespace of materialNamespaces) {  // changed const to let
        namespace = namespace.replace('(','\\(');      // added replace
        namespace = namespace.replace(')','\\)');      // added replace
        for (const name of RENAMED_FUNCTIONS) {
          content = migrateFunction(content, namespace, name, namespace, "m2-" + name);
        }
        for (const name of RENAMED_VARIABLES) {
          content = migrateVariable(content, namespace, name, namespace, "m2-" + name);
        }
      }
...

And migrations went through.