feat(let, return): allow falsy valueChange (#2812)

2025-12-08 19:26:05 +00:00 · 2025-09-08 14:35:07 -07:00 · 2025-09-08 14:35:07 -07:00 · 01967f6cca
commit 01967f6cca
parent 1ba736d092
36 changed files with 138 additions and 40 deletions
--- a/.sizes.json
+++ b/.sizes.json
@ -7,8 +7,8 @@
    {
      "name": "*",
      "total": {
-        "min": 19091,
-        "brotli": 7284
+        "min": 19096,
+        "brotli": 7317
      }
    },
    {
--- a/.sizes/dom.js
+++ b/.sizes/dom.js
@ -1,4 +1,4 @@
-// size: 19091 (min) 7284 (brotli)
+// size: 19096 (min) 7317 (brotli)
 var empty = [],
  rest = Symbol();
 function attrTag(attrs) {
@ -731,7 +731,7 @@ function _var(scope, childAccessor, signal) {
 }
 var _return = (scope, value) => scope.e?.(value);
 function _return_change(scope, changeHandler) {
-  scope.f = changeHandler;
+  changeHandler && (scope.f = changeHandler);
 }
 var _var_change = (scope, value) => scope.f?.(value),
  tagIdsByGlobal = new WeakMap();
--- a/packages/runtime-tags/src/tests/fixtures/bind-to-input/snapshots/html.expected/tags/counter.js
+++ b/packages/runtime-tags/src/tests/fixtures/bind-to-input/snapshots/html.expected/tags/counter.js
@ -14,7 +14,7 @@ export default _._template("__tests__/tags/counter.marko", (input, $serialize) =
    $countChange: _._serialize_if($serialize, /* input.count */2) && $countChange,
    count: _._serialize_if($serialize, /* input.countChange */1) && count,
    x,
-    "TagVariableChange:x": $countChange
+    "TagVariableChange:x": $countChange || void 0
  }, "__tests__/tags/counter.marko", 0, {
    $countChange: 0,
    count: "1:10",
--- a/packages/runtime-tags/src/tests/fixtures/cross-tag-closure/snapshots/dom.expected/template.js
+++ b/packages/runtime-tags/src/tests/fixtures/cross-tag-closure/snapshots/dom.expected/template.js
@ -8,7 +8,7 @@ const $mytag_content__count__script = _._script("__tests__/template.marko_1_coun
    count
  }
 }) => _._on($scope["#button/0"], "click", function () {
-  _._var_change($scope._["#childScope/0"], ++count)
+  _._var_change($scope._["#childScope/0"], ++count, "count")
 }));
 const $mytag_content__count = /* @__PURE__ */_._closure_get("count", ($scope, count) => {
  _._text($scope["#text/1"], count);
--- a/packages/runtime-tags/src/tests/fixtures/cross-tag-closure/snapshots/html.expected/tags/my-let.js
+++ b/packages/runtime-tags/src/tests/fixtures/cross-tag-closure/snapshots/html.expected/tags/my-let.js
@ -6,7 +6,7 @@ export default _._template("__tests__/tags/my-let.marko", input => {
  _._scope($scope0_id, {
    "#TagVariableChange": _._resume(_new_value => {
      value = _new_value;
-    }, "__tests__/tags/my-let.marko_0/valueChange", $scope0_id)
+    }, "__tests__/tags/my-let.marko_0/valueChange", $scope0_id) || void 0
  }, "__tests__/tags/my-let.marko", 0);
  _._resume_branch($scope0_id);
  return $return;
--- a/packages/runtime-tags/src/tests/fixtures/custom-tag-var-assignment/snapshots/dom.expected/template.js
+++ b/packages/runtime-tags/src/tests/fixtures/custom-tag-var-assignment/snapshots/dom.expected/template.js
@ -5,14 +5,14 @@ import * as _ from "@marko/runtime-tags/debug/dom";
 const $count__script = _._script("__tests__/template.marko_0_count", ($scope, {
  count
 }) => _._on($scope["#button/2"], "click", function () {
-  _._var_change($scope["#childScope/0"], ++count)
+  _._var_change($scope["#childScope/0"], ++count, "count")
 }));
 const $count = _._var_resume("__tests__/template.marko_0_count/var", /* @__PURE__ */_._const("count", ($scope, count) => {
  _._text($scope["#text/3"], count);
  $count__script($scope);
 }));
 const $setup__script = _._script("__tests__/template.marko_0", $scope => _._on($scope["#button/4"], "click", function () {
-  _._var_change($scope["#childScope/0"], 0);
+  _._var_change($scope["#childScope/0"], 0, "count");
 }));
 export function $setup($scope) {
  _._var($scope, "#childScope/0", $count);
--- a/packages/runtime-tags/src/tests/fixtures/custom-tag-var-assignment/snapshots/html.expected/tags/counter.js
+++ b/packages/runtime-tags/src/tests/fixtures/custom-tag-var-assignment/snapshots/html.expected/tags/counter.js
@ -9,7 +9,7 @@ export default _._template("__tests__/tags/counter.marko", input => {
    x,
    "#TagVariableChange": _._resume(_new_x => {
      x = _new_x;
-    }, "__tests__/tags/counter.marko_0/valueChange", $scope0_id)
+    }, "__tests__/tags/counter.marko_0/valueChange", $scope0_id) || void 0
  }, "__tests__/tags/counter.marko", 0, {
    x: "1:6"
  });
--- a/packages/runtime-tags/src/tests/fixtures/dynamic-tag-var-assignment/snapshots/dom.expected/template.js
+++ b/packages/runtime-tags/src/tests/fixtures/dynamic-tag-var-assignment/snapshots/dom.expected/template.js
@ -8,7 +8,7 @@ import * as _ from "@marko/runtime-tags/debug/dom";
 const $dynamicTag = /* @__PURE__ */_._dynamic_tag("#text/0", 0, () => $count);
 const $count = _._var_resume("__tests__/template.marko_0_count/var", $scope => {});
 const $setup__script = _._script("__tests__/template.marko_0", $scope => _._on($scope["#button/2"], "click", function () {
-  _._var_change($scope["ConditionalScope:#text/0"], 0);
+  _._var_change($scope["ConditionalScope:#text/0"], 0, "count");
 }));
 export function $setup($scope) {
  $dynamicTag($scope, getCounter());
--- a/packages/runtime-tags/src/tests/fixtures/dynamic-tag-var-assignment/snapshots/html.expected/tags/counter.js
+++ b/packages/runtime-tags/src/tests/fixtures/dynamic-tag-var-assignment/snapshots/html.expected/tags/counter.js
@ -9,7 +9,7 @@ export default _._template("__tests__/tags/counter.marko", input => {
    x,
    "#TagVariableChange": _._resume(_new_x => {
      x = _new_x;
-    }, "__tests__/tags/counter.marko_0/valueChange", $scope0_id)
+    }, "__tests__/tags/counter.marko_0/valueChange", $scope0_id) || void 0
  }, "__tests__/tags/counter.marko", 0, {
    x: "1:6"
  });
--- a/packages/runtime-tags/src/tests/fixtures/error-return-write/snapshots/.name-cache.json
+++ b/packages/runtime-tags/src/tests/fixtures/error-return-write/snapshots/.name-cache.json
@ -0,0 +1,8 @@
+{
+  "vars": {
+    "props": {
+      "$_": "r",
+      "$init": "m"
+    }
+  }
+}
--- a/packages/runtime-tags/src/tests/fixtures/error-return-write/snapshots/csr-sanitized.expected.md
+++ b/packages/runtime-tags/src/tests/fixtures/error-return-write/snapshots/csr-sanitized.expected.md
@ -0,0 +1 @@
+x is a readonly tag variable.
--- a/packages/runtime-tags/src/tests/fixtures/error-return-write/snapshots/csr.expected.md
+++ b/packages/runtime-tags/src/tests/fixtures/error-return-write/snapshots/csr.expected.md
@ -0,0 +1 @@
+x is a readonly tag variable.
--- a/packages/runtime-tags/src/tests/fixtures/error-return-write/snapshots/dom.expected/tags/child.js
+++ b/packages/runtime-tags/src/tests/fixtures/error-return-write/snapshots/dom.expected/tags/child.js
@ -0,0 +1,8 @@
+export const $template = "";
+export const $walks = "";
+import * as _ from "@marko/runtime-tags/debug/dom";
+export function $setup($scope) {
+  _._return($scope, 1);
+  _._return_change($scope, false);
+}
+export default /* @__PURE__ */_._template("__tests__/tags/child.marko", $template, $walks, $setup);
--- a/packages/runtime-tags/src/tests/fixtures/error-return-write/snapshots/dom.expected/template.hydrate.js
+++ b/packages/runtime-tags/src/tests/fixtures/error-return-write/snapshots/dom.expected/template.hydrate.js
@ -0,0 +1,4 @@
+// size: 71 (min) 69 (brotli)
+(_._var_resume("b0", ($scope) => {}),
+  _._script("b1", ($scope) => _._var_change($scope[0], 2)),
+  init());
--- a/packages/runtime-tags/src/tests/fixtures/error-return-write/snapshots/dom.expected/template.js
+++ b/packages/runtime-tags/src/tests/fixtures/error-return-write/snapshots/dom.expected/template.js
@ -0,0 +1,12 @@
+export const $template = _child_template;
+export const $walks = /* beginChildWithVar, _child_walks, endChild */`0${_child_walks}&`;
+import { $setup as _child, $template as _child_template, $walks as _child_walks } from "./tags/child.marko";
+import * as _ from "@marko/runtime-tags/debug/dom";
+const $x = _._var_resume("__tests__/template.marko_0_x/var", $scope => {});
+const $setup__script = _._script("__tests__/template.marko_0", $scope => _._var_change($scope["#childScope/0"], 2, "x"));
+export function $setup($scope) {
+  _._var($scope, "#childScope/0", $x);
+  _child($scope["#childScope/0"]);
+  $setup__script($scope);
+}
+export default /* @__PURE__ */_._template("__tests__/template.marko", $template, $walks, $setup);
--- a/packages/runtime-tags/src/tests/fixtures/error-return-write/snapshots/html.expected/tags/child.js
+++ b/packages/runtime-tags/src/tests/fixtures/error-return-write/snapshots/html.expected/tags/child.js
@ -0,0 +1,9 @@
+import * as _ from "@marko/runtime-tags/debug/html";
+export default _._template("__tests__/tags/child.marko", input => {
+  const $scope0_id = _._scope_id();
+  const $return = 1;
+  _._scope($scope0_id, {
+    "#TagVariableChange": false || void 0
+  }, "__tests__/tags/child.marko", 0);
+  return $return;
+});
--- a/packages/runtime-tags/src/tests/fixtures/error-return-write/snapshots/html.expected/template.js
+++ b/packages/runtime-tags/src/tests/fixtures/error-return-write/snapshots/html.expected/template.js
@ -0,0 +1,12 @@
+import _child from "./tags/child.marko";
+import * as _ from "@marko/runtime-tags/debug/html";
+export default _._template("__tests__/template.marko", input => {
+  const $scope0_id = _._scope_id();
+  const $childScope = _._peek_scope_id();
+  let x = _child({});
+  _._var($scope0_id, "#scopeOffset/1", $childScope, "__tests__/template.marko_0_x/var");
+  _._script($scope0_id, "__tests__/template.marko_0");
+  _._scope($scope0_id, {
+    "#childScope/0": _._existing_scope($childScope)
+  }, "__tests__/template.marko", 0);
+});
--- a/packages/runtime-tags/src/tests/fixtures/error-return-write/snapshots/resume-sanitized.expected.md
+++ b/packages/runtime-tags/src/tests/fixtures/error-return-write/snapshots/resume-sanitized.expected.md
@ -0,0 +1 @@
+x is a readonly tag variable.
--- a/packages/runtime-tags/src/tests/fixtures/error-return-write/snapshots/resume.expected.md
+++ b/packages/runtime-tags/src/tests/fixtures/error-return-write/snapshots/resume.expected.md
@ -0,0 +1 @@
+x is a readonly tag variable.
--- a/packages/runtime-tags/src/tests/fixtures/error-return-write/tags/child.marko
+++ b/packages/runtime-tags/src/tests/fixtures/error-return-write/tags/child.marko
@ -0,0 +1 @@
+<return=1 valueChange=false/>
--- a/packages/runtime-tags/src/tests/fixtures/error-return-write/template.marko
+++ b/packages/runtime-tags/src/tests/fixtures/error-return-write/template.marko
@ -0,0 +1,4 @@
+<child/x/>
+<script>
+  x = 2;
+</script>
--- a/packages/runtime-tags/src/tests/fixtures/error-return-write/test.ts
+++ b/packages/runtime-tags/src/tests/fixtures/error-return-write/test.ts
@ -0,0 +1,3 @@
+export const error_runtime = true;
+export const skip_ssr = true;
+export const skip_resume = false;
--- a/packages/runtime-tags/src/tests/fixtures/let-tag-controllable-child/snapshots/html.expected/tags/child.js
+++ b/packages/runtime-tags/src/tests/fixtures/let-tag-controllable-child/snapshots/html.expected/tags/child.js
@ -11,8 +11,8 @@ export default _._template("__tests__/tags/child.marko", (input, $serialize) =>
    input_valueChange: _._serialize_if($serialize, /* input.value */0) && input.valueChange,
    state,
    otherState,
-    "TagVariableChange:state": input.valueChange,
-    "TagVariableChange:otherState": input["value" + "Change"]
+    "TagVariableChange:state": input.valueChange || void 0,
+    "TagVariableChange:otherState": input["value" + "Change"] || void 0
  }, "__tests__/tags/child.marko", 0, {
    input_value: ["input.value"],
    input_valueChange: ["input.valueChange"],
--- a/packages/runtime-tags/src/tests/fixtures/let-tag-controllable-dynamic-change-handler/snapshots/html.expected/template.js
+++ b/packages/runtime-tags/src/tests/fixtures/let-tag-controllable-dynamic-change-handler/snapshots/html.expected/template.js
@ -13,7 +13,7 @@ export default _._template("__tests__/template.marko", input => {
    x,
    yChange,
    y,
-    "TagVariableChange:y": yChange
+    "TagVariableChange:y": yChange || void 0
  }, "__tests__/template.marko", 0, {
    x: "1:6",
    yChange: "2:6",
--- a/packages/runtime-tags/src/tests/fixtures/let-tag-controllable-id/snapshots/html.expected/template.js
+++ b/packages/runtime-tags/src/tests/fixtures/let-tag-controllable-id/snapshots/html.expected/template.js
@ -12,7 +12,7 @@ export default _._template("__tests__/template.marko", input => {
    x,
    handler,
    y,
-    "TagVariableChange:y": handler
+    "TagVariableChange:y": handler || void 0
  }, "__tests__/template.marko", 0, {
    x: "1:6",
    handler: "2:6",
--- a/packages/runtime-tags/src/tests/fixtures/let-tag-controllable-static/snapshots/html.expected/template.js
+++ b/packages/runtime-tags/src/tests/fixtures/let-tag-controllable-static/snapshots/html.expected/template.js
@ -9,7 +9,7 @@ export default _._template("__tests__/template.marko", input => {
    y,
    "TagVariableChange:y": _._resume(function (newValue) {
      x = newValue + 1;
-    }, "__tests__/template.marko_0/valueChange", $scope0_id)
+    }, "__tests__/template.marko_0/valueChange", $scope0_id) || void 0
  }, "__tests__/template.marko", 0, {
    y: "2:6"
  });
--- a/packages/runtime-tags/src/tests/fixtures/let-tag/snapshots/dom.expected/template.js
+++ b/packages/runtime-tags/src/tests/fixtures/let-tag/snapshots/dom.expected/template.js
@ -16,6 +16,6 @@ const $y = /* @__PURE__ */_._let("y/4", ($scope, y) => {
 });
 export function $setup($scope) {
  $x($scope, 1);
-  $y($scope, 1);
+  $y($scope, 1, false);
 }
 export default /* @__PURE__ */_._template("__tests__/template.marko", $template, $walks, $setup);
--- a/packages/runtime-tags/src/tests/fixtures/let-tag/snapshots/html.expected/template.js
+++ b/packages/runtime-tags/src/tests/fixtures/let-tag/snapshots/html.expected/template.js
@ -7,7 +7,8 @@ export default _._template("__tests__/template.marko", input => {
  _._script($scope0_id, "__tests__/template.marko_0_x_y");
  _._scope($scope0_id, {
    x,
-    y
+    y,
+    "TagVariableChange:y": false || void 0
  }, "__tests__/template.marko", 0, {
    x: "1:6",
    y: "2:6"
--- a/packages/runtime-tags/src/tests/fixtures/let-tag/template.marko
+++ b/packages/runtime-tags/src/tests/fixtures/let-tag/template.marko
@ -1,5 +1,5 @@
 <let/x=1/>
-<let/y=1/>
+<let/y=1 valueChange=false/>

 <button onClick=(() => x = y = x + y)>
  ${x}
--- a/packages/runtime-tags/src/tests/fixtures/multiple-bound-values/snapshots/html.expected/tags/2counters.js
+++ b/packages/runtime-tags/src/tests/fixtures/multiple-bound-values/snapshots/html.expected/tags/2counters.js
@ -13,8 +13,8 @@ export default _._template("__tests__/tags/2counters.marko", (input, $serialize)
    input_count2Change: _._serialize_if($serialize, /* input.count2 */2) && input.count2Change,
    count1,
    count2,
-    "TagVariableChange:count1": input.count1Change,
-    "TagVariableChange:count2": input.count2Change
+    "TagVariableChange:count1": input.count1Change || void 0,
+    "TagVariableChange:count2": input.count2Change || void 0
  }, "__tests__/tags/2counters.marko", 0, {
    input_count1: ["input.count1"],
    input_count1Change: ["input.count1Change"],
--- a/packages/runtime-tags/src/dom/signals.ts
+++ b/packages/runtime-tags/src/dom/signals.ts
@ -239,12 +239,22 @@ export const _return = (scope: Scope, value: unknown) =>

 export function _return_change(
  scope: Scope,
-  changeHandler: (value: unknown) => void,
+  changeHandler?: ((value: unknown) => void) | null | false,
 ) {
-  scope[AccessorProp.TagVariableChange] = changeHandler;
+  if (changeHandler) {
+    scope[AccessorProp.TagVariableChange] = changeHandler;
+  }
 }
-export const _var_change = (scope: Scope, value: unknown) =>
-  scope[AccessorProp.TagVariableChange]?.(value);
+export const _var_change = MARKO_DEBUG
+  ? (scope: Scope, value: unknown, name: string = "This") => {
+      if (typeof scope[AccessorProp.TagVariableChange] !== "function") {
+        throw new TypeError(`${name} is a readonly tag variable.`);
+      }
+
+      scope[AccessorProp.TagVariableChange](value);
+    }
+  : (scope: Scope, value: unknown) =>
+      scope[AccessorProp.TagVariableChange]?.(value);

 const tagIdsByGlobal = new WeakMap<Scope["___global"], number>();
 export function _id({ $global }: Scope) {
--- a/packages/runtime-tags/src/translator/core/let.ts
+++ b/packages/runtime-tags/src/translator/core/let.ts
@ -84,7 +84,7 @@ export default {
        );
    }

-    if (valueChangeAttr && computeNode(valueChangeAttr.value)) {
+    if (valueChangeAttr && computeNode(valueChangeAttr.value)?.value) {
      throw tag
        .get("attributes")
        .find((attr) => attr.node === valueChangeAttr)!
@ -150,7 +150,11 @@ export default {
          setBindingSerializedValue(
            section,
            binding,
-            valueChangeAttr.value,
+            t.logicalExpression(
+              "||",
+              valueChangeAttr.value,
+              t.unaryExpression("void", t.numericLiteral(0)),
+            ),
            getAccessorPrefix().TagVariableChange,
          );
        }
--- a/packages/runtime-tags/src/translator/core/return.ts
+++ b/packages/runtime-tags/src/translator/core/return.ts
@ -92,7 +92,11 @@ export default {
          setSectionSerializedValue(
            section,
            getAccessorProp().TagVariableChange,
-            attrs.valueChange,
+            t.logicalExpression(
+              "||",
+              attrs.valueChange,
+              t.unaryExpression("void", t.numericLiteral(0)),
+            ),
          );
        }

--- a/packages/runtime-tags/src/translator/visitors/tag/custom-tag.ts
+++ b/packages/runtime-tags/src/translator/visitors/tag/custom-tag.ts
@ -13,7 +13,7 @@ import path from "path";

 import { generateUid, generateUidIdentifier } from "../../util/generate-uid";
 import { getTagName } from "../../util/get-tag-name";
-import { isOutputHTML } from "../../util/marko-config";
+import { isOptimize, isOutputHTML } from "../../util/marko-config";
 import {
  analyzeAttributeTags,
  type AttrTagLookup,
@ -148,6 +148,10 @@ export default {
      }

      const varBinding = trackVarReferences(tag, BindingType.derived);
+      const mutatesTagVar = !!(
+        tag.node.var?.type === "Identifier" &&
+        tag.scope.getBinding(tag.node.var.name)?.constantViolations.length
+      );
      if (varBinding) {
        varBinding.scopeOffset = tagExtra[kChildOffsetScopeBinding] =
          createBinding("#scopeOffset", BindingType.dom, section);
@ -162,7 +166,7 @@ export default {
          addBindingSerializeReasonExpr(
            section,
            childScopeBinding,
-            varSerializeReason,
+            mutatesTagVar || varSerializeReason,
          );
        }
      } else {
@ -191,7 +195,7 @@ export default {
          addBindingSerializeReasonExpr(
            section,
            childScopeBinding,
-            varSerializeReason,
+            mutatesTagVar || varSerializeReason,
          );
        }

@ -450,16 +454,21 @@ function translateDOM(tag: t.NodePath<t.MarkoTag>) {
  const inputExport = childExports.input;

  if (node.var) {
+    const varBinding = node.var.extra!.binding!;
    const source = initValue(
      // TODO: support destructuring
-      node.var.extra!.binding!,
+      varBinding,
    );
    source.register = true;
    source.buildAssignment = (valueSection, value) => {
-      return t.callExpression(importRuntime("_var_change"), [
+      const changeArgs = [
        createScopeReadExpression(valueSection, childScopeBinding),
        value,
-      ]);
+      ];
+      if (!isOptimize()) {
+        changeArgs.push(t.stringLiteral(varBinding.name));
+      }
+      return t.callExpression(importRuntime("_var_change"), changeArgs);
    };
    addStatement(
      "render",
--- a/packages/runtime-tags/src/translator/visitors/tag/dynamic-tag.ts
+++ b/packages/runtime-tags/src/translator/visitors/tag/dynamic-tag.ts
@ -11,7 +11,7 @@ import { isEventHandler } from "../../../common/helpers";
 import { WalkCode } from "../../../common/types";
 import { generateUidIdentifier } from "../../util/generate-uid";
 import { getAccessorPrefix } from "../../util/get-accessor-char";
-import { isOutputHTML } from "../../util/marko-config";
+import { isOptimize, isOutputHTML } from "../../util/marko-config";
 import { analyzeAttributeTags } from "../../util/nested-attribute-tags";
 import {
  type Binding,
@ -275,13 +275,14 @@ export default {
        const signal = getSignal(section, nodeBinding, "dynamicTag");
        let tagVarSignal: Signal | undefined;
        if (tag.node.var) {
+          const varBinding = tag.node.var.extra!.binding!;
          tagVarSignal = initValue(
            // TODO: support destructuring
-            tag.node.var.extra!.binding!,
+            varBinding,
          );
          tagVarSignal.register = true;
          tagVarSignal.buildAssignment = (valueSection, value) => {
-            return t.callExpression(importRuntime("_var_change"), [
+            const changeArgs = [
              t.memberExpression(
                getScopeExpression(tagVarSignal!.section, valueSection),
                t.stringLiteral(
@ -291,7 +292,11 @@ export default {
                true,
              ),
              value,
-            ]);
+            ];
+            if (!isOptimize()) {
+              changeArgs.push(t.stringLiteral(varBinding.name));
+            }
+            return t.callExpression(importRuntime("_var_change"), changeArgs);
          };
        }

--- a/packages/runtime-tags/tags/let.d.marko
+++ b/packages/runtime-tags/tags/let.d.marko
@ -2,7 +2,7 @@

 export interface Input<T> {
  value: T;
-  valueChange?: (newValue: T) => void;
+  valueChange?: ((newValue: T) => void) | false | null;
 }

 return=input.value valueChange=input.valueChange!