fix: directives on compat tags

marko-js · Aug 18, 2023 · 705598e · 705598e
1 parent dcc424d
commit 705598e
Show file tree

Hide file tree

Showing 22 changed files with 105 additions and 41 deletions.
diff --git a/.changeset/loud-jobs-repeat.md b/.changeset/loud-jobs-repeat.md
@@ -0,0 +1,6 @@
+---
+"marko-widgets": patch
+"@marko/compat-v4": patch
+---
+
+Ensure all tag migrations happen on exit so that directive migrations can run first.
diff --git a/.changeset/pretty-cars-behave.md b/.changeset/pretty-cars-behave.md
@@ -0,0 +1,7 @@
+---
+"marko-widgets": patch
+"@marko/compat-utils": patch
+"@marko/compat-v4": patch
+---
+
+Only skip directive migrations for specific tags that handle all attributes. Previously any migrated tag was not properly getting directives migrated.
diff --git a/packages/compat-utils/src/tags.ts b/packages/compat-utils/src/tags.ts
@@ -8,7 +8,9 @@ export function getTagFile(tag: t.NodePath<t.MarkoTag>) {
   return def && (def.template || def.renderer);
 }
 
-export function willMigrateTag(tag: t.NodePath<t.MarkoTag>) {
+export function willMigrateTag(
+  tag: t.NodePath<t.MarkoTag>,
+): tag is t.NodePath<t.MarkoTag & { name: t.StringLiteral }> {
   switch (getTagDef(tag)?.taglibId) {
     case "marko-widgets":
     case "@marko/compat-v4":
@@ -18,6 +20,19 @@ export function willMigrateTag(tag: t.NodePath<t.MarkoTag>) {
   }
 }
 
+export function willMigrateAllAttrs(tag: t.NodePath<t.MarkoTag>) {
+  if (willMigrateTag(tag)) {
+    switch (tag.node.name.value) {
+      case "assign":
+      case "invoke":
+      case "var":
+        return true;
+    }
+  }
+
+  return false;
+}
+
 export function getTagNameForTemplatePath(
   ref: t.NodePath<any>,
   request: string,

diff --git a/packages/compat-v4/src/components/await/migrate.ts b/packages/compat-v4/src/components/await/migrate.ts
@@ -7,7 +7,7 @@ import {
 } from "@marko/babel-utils";
 
 export default {
-  enter(tag: t.NodePath<t.MarkoTag>) {
+  exit(tag: t.NodePath<t.MarkoTag>) {
     const firstArg = tag.node.arguments?.[0];
     if (firstArg?.type !== "MarkoParseError") return;
     const match = /^\s*([$a-zA-Z_][0-9a-zA-Z_$]*)\s+from\s+/.exec(

diff --git a/packages/compat-v4/src/components/for/migrate.ts b/packages/compat-v4/src/components/for/migrate.ts
@@ -11,7 +11,7 @@ import {
 import { ForType, parseFor } from "./parse-for";
 
 export default {
-  enter(tag: t.NodePath<t.MarkoTag>) {
+  exit(tag: t.NodePath<t.MarkoTag>) {
     const { node } = tag;
     const args = node.arguments;
     const len = args?.length;

diff --git a/packages/compat-v4/src/components/include/migrate.ts b/packages/compat-v4/src/components/include/migrate.ts
@@ -3,7 +3,7 @@ import { diagnosticDeprecate, diagnosticError } from "@marko/babel-utils";
 import { exprToAttrs, getTagNameForTemplatePath } from "@marko/compat-utils";
 
 export default {
-  enter(tag: t.NodePath<t.MarkoTag>) {
+  exit(tag: t.NodePath<t.MarkoTag>) {
     const args = tag.node.arguments;
     if (!args?.length) {
       diagnosticError(tag, {

diff --git a/packages/compat-v4/src/components/macro-body/migrate.ts b/packages/compat-v4/src/components/macro-body/migrate.ts
@@ -2,7 +2,7 @@ import { types as t } from "@marko/compiler";
 import { diagnosticError } from "@marko/babel-utils";
 
 export default {
-  enter(tag: t.NodePath<t.MarkoTag>) {
+  exit(tag: t.NodePath<t.MarkoTag>) {
     const closestMacro = tag.findParent(isMacroDefinition);
     let hasErrors = false;
 

diff --git a/packages/compat-v4/src/components/unless/migrate.ts b/packages/compat-v4/src/components/unless/migrate.ts
@@ -2,7 +2,7 @@ import { types as t } from "@marko/compiler";
 import { diagnosticDeprecate, diagnosticError } from "@marko/babel-utils";
 
 export default {
-  enter(tag: t.NodePath<t.MarkoTag>) {
+  exit(tag: t.NodePath<t.MarkoTag>) {
     const { node } = tag;
     const name = tag.get("name");
     const args = node.arguments || [];

diff --git a/packages/compat-v4/src/migrate/if-directive.ts b/packages/compat-v4/src/migrate/if-directive.ts
@@ -1,6 +1,6 @@
 import { types as t } from "@marko/compiler";
 import { diagnosticDeprecate } from "@marko/babel-utils";
-import { willMigrateTag } from "@marko/compat-utils";
+import { willMigrateAllAttrs } from "@marko/compat-utils";
 
 export default {
   MarkoAttribute(attr) {
@@ -21,7 +21,7 @@ export default {
     }
 
     const tag = attr.parentPath as t.NodePath<t.MarkoTag>;
-    if (willMigrateTag(tag)) return;
+    if (willMigrateAllAttrs(tag)) return;
 
     diagnosticDeprecate(attr, {
       label: `The "${name}" directive is deprecated. Please use "<${name}>" tag instead. See: https://github.com/marko-js/marko/wiki/Deprecation:-control-flow-attributes`,

diff --git a/packages/compat-v4/src/migrate/include-directive.ts b/packages/compat-v4/src/migrate/include-directive.ts
@@ -1,6 +1,6 @@
 import { types as t } from "@marko/compiler";
 import { diagnosticDeprecate } from "@marko/babel-utils";
-import { willMigrateTag } from "@marko/compat-utils";
+import { willMigrateAllAttrs } from "@marko/compat-utils";
 
 export default {
   MarkoAttribute(attr) {
@@ -11,7 +11,7 @@ export default {
     if (name !== "include" || !args?.length) return;
 
     const tag = attr.parentPath as t.NodePath<t.MarkoTag>;
-    if (willMigrateTag(tag)) return;
+    if (willMigrateAllAttrs(tag)) return;
 
     diagnosticDeprecate(attr, {
       label:

diff --git a/packages/compat-v4/src/migrate/loop-directive.ts b/packages/compat-v4/src/migrate/loop-directive.ts
@@ -1,6 +1,6 @@
 import { types as t } from "@marko/compiler";
 import { diagnosticDeprecate } from "@marko/babel-utils";
-import { willMigrateTag } from "@marko/compat-utils";
+import { willMigrateAllAttrs } from "@marko/compat-utils";
 
 export default {
   MarkoAttribute(attr) {
@@ -18,7 +18,7 @@ export default {
     }
 
     const tag = attr.parentPath as t.NodePath<t.MarkoTag>;
-    if (willMigrateTag(tag)) return;
+    if (willMigrateAllAttrs(tag)) return;
 
     diagnosticDeprecate(attr, {
       label: `The "${name}(x)" directive is deprecated. Please use "<${name}>" tag instead. See: https://github.com/marko-js/marko/wiki/Deprecation:-control-flow-attributes`,

diff --git a/packages/marko-widgets/src/components/async-fragment/migrate.ts b/packages/marko-widgets/src/components/async-fragment/migrate.ts
@@ -6,7 +6,7 @@ import {
 } from "@marko/babel-utils";
 
 export default {
-  enter(tag: t.NodePath<t.MarkoTag>) {
+  exit(tag: t.NodePath<t.MarkoTag>) {
     diagnosticDeprecate(tag, {
       label: `The "<async-fragment>" tag is deprecated. Please use "<await>" instead. See: https://github.com/marko-js/marko/wiki/Deprecation:-async-fragment`,
       fix() {

diff --git a/packages/marko-widgets/src/components/async-fragments/migrate.ts b/packages/marko-widgets/src/components/async-fragments/migrate.ts
@@ -1,11 +1,13 @@
 import { types as t } from "@marko/compiler";
 import { diagnosticDeprecate } from "@marko/babel-utils";
 
-export default (tag: t.NodePath<t.MarkoTag>) => {
-  diagnosticDeprecate(tag, {
-    label: "The <async-fragments> is no longer necessary and can be removed.",
-    fix() {
-      tag.replaceWithMultiple(tag.node.body.body);
-    },
-  });
+export default {
+  exit(tag: t.NodePath<t.MarkoTag>) {
+    diagnosticDeprecate(tag, {
+      label: "The <async-fragments> is no longer necessary and can be removed.",
+      fix() {
+        tag.replaceWithMultiple(tag.node.body.body);
+      },
+    });
+  },
 };
diff --git a/packages/marko-widgets/src/components/component-globals/migrate.ts b/packages/marko-widgets/src/components/component-globals/migrate.ts
@@ -1,11 +1,14 @@
 import { types as t } from "@marko/compiler";
 import { diagnosticDeprecate } from "@marko/babel-utils";
 
-export default (tag: t.NodePath<t.MarkoTag>) => {
-  diagnosticDeprecate(tag, {
-    label: "The <component-globals> is no longer necessary and can be removed.",
-    fix() {
-      tag.remove();
-    },
-  });
+export default {
+  exit(tag: t.NodePath<t.MarkoTag>) {
+    diagnosticDeprecate(tag, {
+      label:
+        "The <component-globals> is no longer necessary and can be removed.",
+      fix() {
+        tag.remove();
+      },
+    });
+  },
 };
diff --git a/packages/marko-widgets/src/components/init-widgets/migrate.ts b/packages/marko-widgets/src/components/init-widgets/migrate.ts
@@ -1,11 +1,13 @@
 import { types as t } from "@marko/compiler";
 import { diagnosticDeprecate } from "@marko/babel-utils";
 
-export default (tag: t.NodePath<t.MarkoTag>) => {
-  diagnosticDeprecate(tag, {
-    label: "The <init-widgets> is no longer necessary and can be removed.",
-    fix() {
-      tag.remove();
-    },
-  });
+export default {
+  exit(tag: t.NodePath<t.MarkoTag>) {
+    diagnosticDeprecate(tag, {
+      label: "The <init-widgets> is no longer necessary and can be removed.",
+      fix() {
+        tag.remove();
+      },
+    });
+  },
 };
diff --git a/tests/fixtures-widget/layout-use-tag/__snapshots__/auto-migrate.expected/template.marko b/tests/fixtures-widget/layout-use-tag/__snapshots__/auto-migrate.expected/template.marko
@@ -5,6 +5,12 @@ $ var someData = {
 <${Test}/>
 <${Test} x=1/>
 <${Test} ...someData/>
+<if(someData.y > 2)>
+  <${Test}/>
+</if>
+<if(someData.y <= 2)>
+  <${Test}/>
+</if>
 <test/>
 <test x=1/>
 <test ...someData/>
diff --git a/tests/fixtures-widget/layout-use-tag/__snapshots__/dom.expected.md b/tests/fixtures-widget/layout-use-tag/__snapshots__/dom.expected.md
@@ -13,6 +13,9 @@
   "y": 2
 }
 </div>
+<div>
+  test.marko {}
+</div>
 <div>
   components/test.marko {}
 </div>

diff --git a/tests/fixtures-widget/layout-use-tag/__snapshots__/dom.expected/template.js b/tests/fixtures-widget/layout-use-tag/__snapshots__/dom.expected/template.js
@@ -18,11 +18,17 @@ _marko_template._ = _marko_renderer(function (input, out, _componentDef, _compon
     "x": 1
   }, out, _componentDef, "1");
   _marko_tag(Test, someData, out, _componentDef, "2");
-  _marko_tag(_test, {}, out, _componentDef, "3");
+  if (someData.y > 2) {
+    _marko_tag(Test, {}, out, _componentDef, "3");
+  }
+  if (someData.y <= 2) {
+    _marko_tag(Test, {}, out, _componentDef, "4");
+  }
+  _marko_tag(_test, {}, out, _componentDef, "5");
   _marko_tag(_test, {
     "x": 1
-  }, out, _componentDef, "4");
-  _marko_tag(_test, someData, out, _componentDef, "5");
+  }, out, _componentDef, "6");
+  _marko_tag(_test, someData, out, _componentDef, "7");
 }, {
   t: _marko_componentType,
   i: true,

diff --git a/tests/fixtures-widget/layout-use-tag/__snapshots__/html.expected.md b/tests/fixtures-widget/layout-use-tag/__snapshots__/html.expected.md
@@ -3,7 +3,7 @@
     "x": 1
   }</div><div>test.marko {
     "y": 2
-  }</div><div>components/test.marko {}</div><div>components/test.marko {
+  }</div><div>test.marko {}</div><div>components/test.marko {}</div><div>components/test.marko {
     "x": 1
   }</div><div>components/test.marko {
     "y": 2
@@ -24,6 +24,9 @@
   "y": 2
 }
 </div>
+<div>
+  test.marko {}
+</div>
 <div>
   components/test.marko {}
 </div>

diff --git a/tests/fixtures-widget/layout-use-tag/__snapshots__/html.expected/template.js b/tests/fixtures-widget/layout-use-tag/__snapshots__/html.expected/template.js
@@ -16,11 +16,17 @@ _marko_template._ = _marko_renderer(function (input, out, _componentDef, _compon
     "x": 1
   }, out, _componentDef, "1");
   _marko_tag(Test, someData, out, _componentDef, "2");
-  _marko_tag(_test, {}, out, _componentDef, "3");
+  if (someData.y > 2) {
+    _marko_tag(Test, {}, out, _componentDef, "3");
+  }
+  if (someData.y <= 2) {
+    _marko_tag(Test, {}, out, _componentDef, "4");
+  }
+  _marko_tag(_test, {}, out, _componentDef, "5");
   _marko_tag(_test, {
     "x": 1
-  }, out, _componentDef, "4");
-  _marko_tag(_test, someData, out, _componentDef, "5");
+  }, out, _componentDef, "6");
+  _marko_tag(_test, someData, out, _componentDef, "7");
 }, {
   t: _marko_componentType,
   i: true,

diff --git a/tests/fixtures-widget/layout-use-tag/__snapshots__/hydrate.expected.md b/tests/fixtures-widget/layout-use-tag/__snapshots__/hydrate.expected.md
@@ -13,6 +13,9 @@
   "y": 2
 }
 </div>
+<div>
+  test.marko {}
+</div>
 <div>
   components/test.marko {}
 </div>

diff --git a/tests/fixtures-widget/layout-use-tag/template.marko b/tests/fixtures-widget/layout-use-tag/template.marko
@@ -3,6 +3,8 @@
 <layout-use("./test.marko")/>
 <layout-use("./test.marko", { x: 1 })/>
 <layout-use("./test.marko", someData)/>
+<layout-use("./test.marko") if(someData.y > 2)/>
+<layout-use("./test.marko") if(someData.y <= 2)/>
 
 <layout-use("./components/test.marko")/>
 <layout-use("./components/test.marko", { x: 1 })/>
-Original file line number
+Diff line change
@@ Expand Up / @@ -13,6 +13,9 @@ @@
       "y": 2
     }
     </div>
+    <div>
+      test.marko {}
+    </div>
     <div>
       components/test.marko {}
     </div>
@@ Expand Down @@