Fix infinite loop during css replacement

origami-z · Feb 2, 2024 · 90f7420 · 90f7420
1 parent 4e8eade
commit 90f7420
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 23 deletions.
diff --git a/__tests__/migration/utils.spec.js b/__tests__/migration/utils.spec.js
@@ -5,6 +5,7 @@ import {
   moveNamedImports,
   renameReactElementName,
   replaceReactAttribute,
+  migrateCssVar,
 } from "../../migration/utils.js";
 
 /**
@@ -18,19 +19,51 @@ function createFileWithContent(sourceFileString) {
   return file;
 }
 
-describe("getCssRenameCheckRegex", () => {
-  test("regex match var intended", () => {
-    const actual = getCssRenameCheckRegex(
-      new Map([["--salt-a", "--salt-a-new"]])
-    );
-    expect(actual.test("--my-var: var(--salt-a);")).toBe(true);
-    expect(actual.test("--my-var: var(--salt-b);")).toBe(false);
+describe("CSS migration", () => {
+  const cssMap = new Map([
+    ["--salt-a", "--salt-a-new"],
+    ["--salt-b-1", "--salt-b-100"],
+  ]);
+
+  describe("getCssRenameCheckRegex", () => {
+    test("regex match var intended", () => {
+      const actual = getCssRenameCheckRegex(cssMap);
+      expect(actual.test("--my-var: var(--salt-a);")).toBe(true);
+      actual.lastIndex = 0;
+      expect(actual.test("--my-var: var(--salt-b-1);")).toBe(true);
+      actual.lastIndex = 0;
+    });
+    test("regex does not match var with other suffix", () => {
+      const actual = getCssRenameCheckRegex(cssMap);
+      expect(actual.test("--my-var: var(--salt-a-extra);")).toBe(false);
+      actual.lastIndex = 0;
+      expect(actual.test("--my-var: var(--salt-b);")).toBe(false);
+      actual.lastIndex = 0;
+      expect(actual.test("--my-var: var(--salt-b-100);")).toBe(false);
+      actual.lastIndex = 0;
+    });
   });
-  test("regex does not match var with other suffix", () => {
-    const actual = getCssRenameCheckRegex(
-      new Map([["--salt-a", "--salt-a-new"]])
-    );
-    expect(actual.test("--my-var: var(--salt-a-extra);")).toBe(false);
+
+  describe("migrateCssVar", () => {
+    const regexCheck = getCssRenameCheckRegex(cssMap);
+    test("migrate all instances of the same var", () => {
+      const actual = migrateCssVar(
+        "--my-var: var(--salt-a); --my-var: var(--salt-a);",
+        regexCheck,
+        cssMap
+      );
+      expect(actual).toEqual(
+        "--my-var: var(--salt-a-new); --my-var: var(--salt-a-new);"
+      );
+    });
+    test("does not result in a infinite loop when new value includes old value", () => {
+      const actual = migrateCssVar(
+        "--my-var: var(--salt-b-1);",
+        regexCheck,
+        cssMap
+      );
+      expect(actual).toEqual("--my-var: var(--salt-b-100);");
+    });
   });
 });
 

diff --git a/migration/utils.js b/migration/utils.js
@@ -1,9 +1,18 @@
 import { SyntaxKind, Node } from "ts-morph";
 import { verboseOnlyDimLog, verboseOnlyLog } from "../utils/log.js";
 
+/**
+ * @typedef {Object} RenameImportModuleSpecifierOption
+ * @property {string} from package name to be renamed from
+ * @property {string} to package name rename to
+ * @property {boolean} partial whether rename on partial match
+ */
+
 /**
  *
  * @param {import("ts-morph").ImportDeclaration} declaration
+ * @param {RenameImportModuleSpecifierOption} option
+ * @returns
  */
 export function renameImportModuleSpecifier(
   declaration,
@@ -278,12 +287,14 @@ export function warnUnknownSaltThemeVars(
 }
 
 /**
+ * !WARN: Be mindful of regex side effect when reusing regex
+ * @link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/lastIndex#avoiding_side_effects
  *
  * @param {Map<string, string>} cssMigrationMap
  * @returns {RegExp}
  */
 export function getCssRenameCheckRegex(cssMigrationMap) {
-  const varEndDetector = "(?!-)";
+  const varEndDetector = "(?![\\w-])";
   return new RegExp(
     "(" + Array.from(cssMigrationMap.keys()).join("|") + ")" + varEndDetector,
     "g"
@@ -297,14 +308,9 @@ export function getCssRenameCheckRegex(cssMigrationMap) {
  * @param {Map<string, string>} renameMap
  */
 export function migrateCssVar(line, renameRegex, renameMap) {
-  let result = line;
-  let match = result.match(renameRegex);
-  while (match) {
-    const from = match[0];
-    const to = renameMap.get(from);
-    verboseOnlyLog("Replace css var", from, "to", to);
-    result = result.replace(from, to);
-    match = result.match(renameRegex);
-  }
-  return result;
+  return line.replaceAll(renameRegex, (match, offset, string) => {
+    const to = renameMap.get(match);
+    verboseOnlyLog("Replace css var", match, "to", to);
+    return to;
+  });
 }