fix: improve non standard template literal processing

marko-js · Nov 11, 2024 · a62ac60 · a62ac60
1 parent 132211e
commit a62ac60
Show file tree

Hide file tree

Showing 12 changed files with 216 additions and 89 deletions.
diff --git a/.changeset/modern-feet-move.md b/.changeset/modern-feet-move.md
@@ -0,0 +1,7 @@
+---
+"@marko/compat-v4": patch
+"@marko/compat-utils": patch
+"marko-widgets": patch
+---
+
+Improve handling of non standard template literals.
diff --git a/packages/compat-v4/src/migrate/non-standard-template-literals/index.ts b/packages/compat-v4/src/migrate/non-standard-template-literals/index.ts
@@ -45,62 +45,94 @@ export function migrateNonStandardTemplateLiterals(path: t.NodePath<t.Node>) {
 }
 
 function StringLiteral(string: t.NodePath<t.StringLiteral>) {
-  const templateLiteral = parseNonStandardTemplateLiteral(string);
-  if (templateLiteral) {
-    if (
-      templateLiteral.expressions.length === 1 &&
-      templateLiteral.quasis.length === 2 &&
-      templateLiteral.quasis[0].value.raw === "" &&
-      templateLiteral.quasis[1].value.raw === ""
-    ) {
-      diagnosticDeprecate(string, {
-        label: "Non-standard template literals are deprecated.",
-        fix() {
-          string.replaceWith(templateLiteral.expressions[0]);
-        },
-      });
-    } else if (templateLiteral.expressions.every(isNotNullish)) {
-      diagnosticDeprecate(string, {
-        label: "Non-standard template literals are deprecated.",
-        fix() {
-          string.replaceWith(templateLiteral);
-        },
-      });
+  const { file } = string.hub;
+  const replacement = parseAllNonStandardTemplateLiterals(file, string.node);
+  if (replacement) {
+    if (t.isTemplateLiteral(replacement)) {
+      if (replacement.expressions.every(isNotNullish)) {
+        diagnosticDeprecate(string, {
+          label: "Non-standard template literals are deprecated.",
+          fix() {
+            string.replaceWith(replacement);
+          },
+        });
+      } else {
+        diagnosticDeprecate(string, {
+          label: "Non-standard template literals are deprecated.",
+          fix: {
+            type: "confirm",
+            message:
+              "Are the interpolated values guaranteed to not be null or undefined?",
+            apply(confirm) {
+              if (confirm) {
+                string.replaceWith(replacement);
+              } else {
+                string.replaceWith(
+                  t.templateLiteral(
+                    replacement.quasis,
+                    replacement.expressions.map((expr) => {
+                      return isNotNullish(expr)
+                        ? expr
+                        : castNullishToString(file, expr as t.Expression);
+                    }),
+                  ),
+                );
+              }
+            },
+          },
+        });
+      }
     } else {
       diagnosticDeprecate(string, {
         label: "Non-standard template literals are deprecated.",
-        fix: {
-          type: "confirm",
-          message:
-            "Are the interpolated values guaranteed to not be null or undefined?",
-          apply(confirm) {
-            if (confirm) {
-              string.replaceWith(templateLiteral);
-            } else {
-              string.replaceWith(
-                t.templateLiteral(
-                  templateLiteral.quasis,
-                  templateLiteral.expressions.map((expr) => {
-                    return isNotNullish(expr)
-                      ? expr
-                      : castNullishToString(string, expr as t.Expression);
-                  }),
-                ),
-              );
-            }
-          },
+        fix() {
+          string.replaceWith(replacement);
         },
       });
     }
   }
 }
 
-function castNullishToString(string: t.NodePath, expression: t.Expression) {
-  let nullishHelper = nullishHelpers.get(string.hub);
+function parseAllNonStandardTemplateLiterals(
+  file: t.BabelFile,
+  node: t.StringLiteral,
+) {
+  const templateLiteral = parseNonStandardTemplateLiteral(file, node);
+  if (templateLiteral) {
+    for (let i = templateLiteral.expressions.length; i--; ) {
+      traverseWithParent(
+        file,
+        templateLiteral.expressions[i],
+        templateLiteral,
+        i,
+        replaceNestedNonStandardTemplateLiteral,
+      );
+    }
+
+    return isSingleExpressionTemplateLiteral(templateLiteral)
+      ? templateLiteral.expressions[0]
+      : templateLiteral;
+  }
+}
+
+function replaceNestedNonStandardTemplateLiteral(
+  file: t.BabelFile,
+  node: t.Node,
+  parent: t.Node,
+  key: string | number,
+) {
+  if (node.type === "StringLiteral") {
+    (parent as any)[key] =
+      parseAllNonStandardTemplateLiterals(file, node) || node;
+  }
+}
+
+function castNullishToString(file: t.BabelFile, expression: t.Expression) {
+  let nullishHelper = nullishHelpers.get(file.hub);
   if (!nullishHelper) {
-    nullishHelper = string.scope.generateUidIdentifier("toString");
-    nullishHelpers.set(string.hub, nullishHelper);
-    string.hub.file.path.unshiftContainer(
+    nullishHelper = file.path.scope.generateUidIdentifier("toString");
+    nullishHelpers.set(file.hub, nullishHelper);
+    file.path.unshiftContainer(
       "body",
       t.markoScriptlet(
         [
@@ -129,6 +161,15 @@ function castNullishToString(string: t.NodePath, expression: t.Expression) {
   return t.callExpression(nullishHelper, [expression]);
 }
 
+function isSingleExpressionTemplateLiteral(templateLiteral: t.TemplateLiteral) {
+  return (
+    templateLiteral.expressions.length === 1 &&
+    templateLiteral.quasis.length === 2 &&
+    templateLiteral.quasis[0].value.raw === "" &&
+    templateLiteral.quasis[1].value.raw === ""
+  );
+}
+
 function isNotNullish(node: t.Node): boolean {
   switch (node.type) {
     case "ArrayExpression":
@@ -162,3 +203,35 @@ function isNotNullish(node: t.Node): boolean {
       return false;
   }
 }
+
+function traverseWithParent(
+  file: t.BabelFile,
+  node: t.Node | null | undefined,
+  parent: t.Node,
+  key: string | number,
+  enter: (
+    file: t.BabelFile,
+    node: t.Node,
+    parent: t.Node,
+    key: string | number,
+  ) => void,
+): void {
+  if (!node) return;
+
+  const keys = (t as any).VISITOR_KEYS[node.type];
+  if (!keys) return;
+
+  enter(file, node, parent, key);
+
+  for (const key of keys) {
+    const value: t.Node | undefined | null = (node as any)[key];
+
+    if (Array.isArray(value)) {
+      for (let i = 0; i < value.length; i++) {
+        traverseWithParent(file, value[i], value, i, enter);
+      }
+    } else {
+      traverseWithParent(file, value, parent, key, enter);
+    }
+  }
+}
diff --git a/packages/compat-v4/src/migrate/non-standard-template-literals/parse.ts b/packages/compat-v4/src/migrate/non-standard-template-literals/parse.ts
@@ -17,18 +17,19 @@ const enum CODE {
   OPEN_PAREN = 40,
   OPEN_SQUARE_BRACKET = 91,
   SINGLE_QUOTE = 39,
+  EXCLAMATION = 33,
 }
 
 export function parseNonStandardTemplateLiteral(
-  string: t.NodePath<t.StringLiteral>,
+  file: t.BabelFile,
+  string: t.StringLiteral,
 ) {
-  const { file } = string.hub;
-  const { extra } = string.node;
+  const { extra } = string;
   let value = extra?.raw as string | undefined;
   if (typeof value !== "string") return;
   value = value.slice(1, -1);
   const { length } = value;
-  const nodeStart = getStart(file, string.node);
+  const nodeStart = getStart(file, string);
   const valueStart = nodeStart == null ? null : nodeStart + 1;
   let elements: undefined | t.TemplateElement[];
   let expressions: undefined | t.Expression[];
@@ -39,9 +40,16 @@ export function parseNonStandardTemplateLiteral(
       case CODE.BACK_SLASH:
         i++;
         break;
-      case CODE.DOLLAR_SIGN:
-        if (value.charCodeAt(i + 1) === CODE.OPEN_CURLY_BRACE) {
-          const bracketStart = i + 2;
+      case CODE.DOLLAR_SIGN: {
+        const bracketOffset =
+          value.charCodeAt(i + 1) === CODE.EXCLAMATION &&
+          value.charCodeAt(i + 2) === CODE.OPEN_CURLY_BRACE
+            ? 3
+            : value.charCodeAt(i + 1) === CODE.OPEN_CURLY_BRACE
+              ? 2
+              : 0;
+        if (bracketOffset) {
+          const bracketStart = i + bracketOffset;
           const bracketEnd = skipBracketed(
             value,
             bracketStart,
@@ -56,25 +64,31 @@ export function parseNonStandardTemplateLiteral(
 
           i = bracketEnd - 1;
           lastEndBracket = bracketEnd;
-          const expr =
-            valueStart != null
-              ? parseExpression(
-                  file,
-                  value.slice(bracketStart, i),
-                  valueStart + bracketStart,
-                  valueStart + i,
-                )
-              : parseExpression(file, value.slice(bracketStart, i));
-
-          if (elements) {
-            elements.push(el);
-            expressions!.push(expr);
-          } else {
-            elements = [el];
-            expressions = [expr];
+
+          try {
+            const expr =
+              valueStart != null
+                ? parseExpression(
+                    file,
+                    value.slice(bracketStart, i),
+                    valueStart + bracketStart,
+                    valueStart + i,
+                  )
+                : parseExpression(file, value.slice(bracketStart, i));
+
+            if (elements) {
+              elements.push(el);
+              expressions!.push(expr);
+            } else {
+              elements = [el];
+              expressions = [expr];
+            }
+          } catch {
+            return; // bail if we couldn't process the expression.
           }
         }
         break;
+      }
     }
   }
 

diff --git a/...s-class/non-standard-template-literals/__snapshots__/auto-migrate.expected/template.marko b/...s-class/non-standard-template-literals/__snapshots__/auto-migrate.expected/template.marko
@@ -40,7 +40,10 @@ $ const d = "d";
 <div id="j">
   ${'\${abc}'}
 </div>
-<div ...a id="c"/>
-<div ...d id="c"/>
+<div ...a id="k"/>
+<div ...d id="l"/>
+<div id="m">
+  ${a}
+</div>
 $ const handler = console.log;
 <button onClick(handler)/>
diff --git a/tests/fixtures-class/non-standard-template-literals/__snapshots__/dom.expected.md b/tests/fixtures-class/non-standard-template-literals/__snapshots__/dom.expected.md
@@ -55,11 +55,16 @@
   ${abc}
 </div>
 <div
-  id="c"
+  id="k"
 />
 <div
   d=""
-  id="c"
+  id="l"
 />
+<div
+  id="m"
+>
+  1
+</div>
 <button />
 ```
diff --git a/tests/fixtures-class/non-standard-template-literals/__snapshots__/dom.expected/template.js b/tests/fixtures-class/non-standard-template-literals/__snapshots__/dom.expected/template.js
@@ -76,13 +76,18 @@ _marko_template._ = _marko_renderer(function (input, out, _componentDef, _compon
   out.t('\${abc}', _component);
   out.ee();
   out.e("div", _marko_merge_attrs(a, {
-    "id": "c"
+    "id": "k"
   }), "11", _component, 0, 4);
   out.e("div", _marko_merge_attrs(d, {
-    "id": "c"
+    "id": "l"
   }), "12", _component, 0, 4);
+  out.be("div", {
+    "id": "m"
+  }, "13", _component, null, 1);
+  out.t(a, _component);
+  out.ee();
   const handler = console.log;
-  out.e("button", null, "13", _component, 0, 0, {
+  out.e("button", null, "14", _component, 0, 0, {
     "onclick": _componentDef.d("click", handler, false)
   });
 }, {

diff --git a/tests/fixtures-class/non-standard-template-literals/__snapshots__/html.expected.md b/tests/fixtures-class/non-standard-template-literals/__snapshots__/html.expected.md
@@ -1,5 +1,5 @@
 # Write
-  <!--M#s0-0--><div id="1" data-other=2></div><div id=a>${STATIC}</div><div id=b>${SCRIPLET}</div><div id=c>1</div><div id=d>abc}</div><div id=e>abc}</div><div id=f>abc}</div><div id=g>abcd}ef</div><div id=h>abc3</div><div id=i>abcdef</div><div id=j>${abc}</div><div id=c></div><div id=c d></div><button></button><!--M/--><script>$MC=(window.$MC||[]).concat({"w":[["s0-0",0,{"renderBody":null},{"f":1,"r":null}]],"t":["<fixture-dir>/template.marko"]})</script>
+  <!--M#s0-0--><div id="1" data-other=2></div><div id=a>${STATIC}</div><div id=b>${SCRIPLET}</div><div id=c>1</div><div id=d>abc}</div><div id=e>abc}</div><div id=f>abc}</div><div id=g>abcd}ef</div><div id=h>abc3</div><div id=i>abcdef</div><div id=j>${abc}</div><div id=k></div><div id=l d></div><div id=m>1</div><button></button><!--M/--><script>$MC=(window.$MC||[]).concat({"w":[["s0-0",0,{"renderBody":null},{"f":1,"r":null}]],"t":["<fixture-dir>/template.marko"]})</script>
 
 # Render
 ```html
@@ -58,11 +58,16 @@
   ${abc}
 </div>
 <div
-  id="c"
+  id="k"
 />
 <div
   d=""
-  id="c"
+  id="l"
 />
+<div
+  id="m"
+>
+  1
+</div>
 <button />
 ```
diff --git a/tests/fixtures-class/non-standard-template-literals/__snapshots__/html.expected/template.js b/tests/fixtures-class/non-standard-template-literals/__snapshots__/html.expected/template.js
@@ -53,11 +53,14 @@ _marko_template._ = _marko_renderer(function (input, out, _componentDef, _compon
   out.w("${abc}");
   out.w("</div>");
   out.w(`<div${_marko_merge_attrs(a, {
-    "id": "c"
+    "id": "k"
   })}></div>`);
   out.w(`<div${_marko_merge_attrs(d, {
-    "id": "c"
+    "id": "l"
   })}></div>`);
+  out.w("<div id=m>");
+  out.w(_marko_escapeXml(a));
+  out.w("</div>");
   const handler = console.log;
   out.w("<button></button>");
 }, {
-Original file line number
+Diff line change
@@ Expand Up / @@ -55,11 +55,16 @@ @@
       ${abc}
     </div>
     <div
-      id="c"
+      id="k"
     />
     <div
       d=""
-      id="c"
+      id="l"
     />
+    <div
+      id="m"
+    >
+    </div>
     <button />
     ```