Skip to content

Commit

Permalink
fix: restore native tag state before change handlers are called
Browse files Browse the repository at this point in the history
  • Loading branch information
DylanPiercey committed Feb 14, 2025
1 parent f912994 commit f99c90f
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 138 deletions.
5 changes: 5 additions & 0 deletions .changeset/strange-avocados-study.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@marko/runtime-tags": patch
---

Restore original native tag internal state before calling change handlers.
4 changes: 2 additions & 2 deletions .sizes.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
{
"name": "*",
"total": {
"min": 18176,
"brotli": 6819
"min": 18072,
"brotli": 6823
}
},
{
Expand Down
107 changes: 48 additions & 59 deletions .sizes/dom.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// size: 18176 (min) 6819 (brotli)
// size: 18072 (min) 6823 (brotli)
var empty = [],
rest = Symbol();
function attrTag(attrs2) {
Expand Down Expand Up @@ -289,11 +289,10 @@ function controllable_input_checked_effect(scope, nodeAccessor) {
let el = scope[nodeAccessor];
syncControllable(el, "input", hasCheckboxChanged, () => {
let checkedChange = scope[nodeAccessor + ";"];
checkedChange &&
((scope[nodeAccessor + "="] = 6),
checkedChange(el.checked),
run(),
6 === scope[nodeAccessor + "="] && (el.checked = !el.checked));
if (checkedChange) {
let newValue = el.checked;
(el.checked = !newValue), checkedChange(newValue), run();
}
});
}
function controllable_input_checkedValue(
Expand All @@ -320,36 +319,30 @@ function controllable_input_checkedValue_effect(scope, nodeAccessor) {
syncControllable(el, "input", hasCheckboxChanged, () => {
let checkedValueChange = scope[nodeAccessor + ";"];
if (checkedValueChange) {
let oldValue = scope[nodeAccessor + ":"];
if (
((scope[nodeAccessor + "="] = 6),
checkedValueChange(
Array.isArray(oldValue)
? (function (arr, val, push) {
let index = arr.indexOf(val);
return (
(push
? !~index && [...arr, val]
: ~index &&
arr.slice(0, index).concat(arr.slice(index + 1))) || arr
);
})(oldValue, el.value, el.checked)
: el.checked
? el.value
: void 0,
),
run(),
6 === scope[nodeAccessor + "="])
)
if (el.name && "r" === el.type[0])
for (let radio of el
.getRootNode()
.querySelectorAll(`[type=radio][name=${CSS.escape(el.name)}]`))
radio.form === el.form &&
(radio.checked = Array.isArray(oldValue)
? oldValue.includes(radio.value)
: oldValue === radio.value);
else el.checked = !el.checked;
let oldValue = scope[nodeAccessor + ":"],
newValue = Array.isArray(oldValue)
? (function (arr, val, push) {
let index = arr.indexOf(val);
return (
(push
? !~index && [...arr, val]
: ~index &&
arr.slice(0, index).concat(arr.slice(index + 1))) || arr
);
})(oldValue, el.value, el.checked)
: el.checked
? el.value
: void 0;
if (el.name && "r" === el.type[0])
for (let radio of el
.getRootNode()
.querySelectorAll(`[type=radio][name=${CSS.escape(el.name)}]`))
radio.form === el.form &&
(radio.checked = Array.isArray(oldValue)
? oldValue.includes(radio.value)
: oldValue === radio.value);
else el.checked = !el.checked;
checkedValueChange(newValue), run();
}
});
}
Expand All @@ -370,14 +363,14 @@ function controllable_input_value_effect(scope, nodeAccessor) {
isResuming && (scope[nodeAccessor + ":"] = el.defaultValue),
syncControllable(el, "input", hasValueChanged, (ev) => {
let valueChange = scope[nodeAccessor + ";"];
valueChange &&
((scope[nodeAccessor + "="] = 6),
ev && (inputType = ev.inputType),
valueChange(el.value),
run(),
6 === scope[nodeAccessor + "="] &&
if (valueChange) {
let newValue = el.value;
(inputType = ev?.inputType),
setValueAndUpdateSelection(el, scope[nodeAccessor + ":"]),
(inputType = ""));
valueChange(newValue),
run(),
(inputType = "");
}
});
}
function controllable_select_value(scope, nodeAccessor, value2, valueChange) {
Expand All @@ -393,16 +386,14 @@ function controllable_select_value_effect(scope, nodeAccessor) {
let el = scope[nodeAccessor],
onChange = () => {
let valueChange = scope[nodeAccessor + ";"];
valueChange &&
((scope[nodeAccessor + "="] = 6),
valueChange(
Array.isArray(scope[nodeAccessor + ":"])
? Array.from(el.selectedOptions, toValueProp)
: el.value,
),
run(),
6 === scope[nodeAccessor + "="] &&
setSelectOptions(el, scope[nodeAccessor + ":"], valueChange));
if (valueChange) {
let newValue = Array.isArray(scope[nodeAccessor + ":"])
? Array.from(el.selectedOptions, toValueProp)
: el.value;
setSelectOptions(el, scope[nodeAccessor + ":"], valueChange),
valueChange(newValue),
run();
}
};
el._ ||
new MutationObserver(() => {
Expand Down Expand Up @@ -450,12 +441,10 @@ function controllable_detailsOrDialog_open_effect(scope, nodeAccessor) {
hasChanged,
() => {
let openChange = scope[nodeAccessor + ";"];
openChange &&
hasChanged() &&
((scope[nodeAccessor + "="] = 6),
openChange(el.open),
run(),
6 === scope[nodeAccessor + "="] && (el.open = !el.open));
if (openChange && hasChanged()) {
let newValue = el.open;
(el.open = !newValue), openChange(newValue), run();
}
},
);
}
Expand Down
120 changes: 43 additions & 77 deletions packages/runtime-tags/src/dom/controllable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,10 @@ export function controllable_input_checked_effect(
nodeAccessor + AccessorChar.ControlledHandler
] as undefined | ((value: unknown) => unknown);
if (checkedChange) {
scope[nodeAccessor + AccessorChar.ControlledType] =
ControlledType.Pending;
checkedChange(el.checked);
const newValue = el.checked;
el.checked = !newValue;
checkedChange(newValue);
run();
if (
scope[nodeAccessor + AccessorChar.ControlledType] ===
ControlledType.Pending
) {
el.checked = !el.checked;
}
}
});
}
Expand Down Expand Up @@ -78,37 +72,29 @@ export function controllable_input_checkedValue_effect(
] as undefined | ((value: unknown) => unknown);
if (checkedValueChange) {
const oldValue = scope[nodeAccessor + AccessorChar.ControlledValue];
scope[nodeAccessor + AccessorChar.ControlledType] =
ControlledType.Pending;
checkedValueChange(
Array.isArray(oldValue)
? updateList(oldValue, el.value, el.checked)
: el.checked
? el.value
: undefined,
);
run();

if (
scope[nodeAccessor + AccessorChar.ControlledType] ===
ControlledType.Pending
) {
if (el.name && el.type[0] === "r") {
for (const radio of (
el.getRootNode() as Document | ShadowRoot
).querySelectorAll<HTMLInputElement>(
`[type=radio][name=${CSS.escape(el.name)}]`,
)) {
if (radio.form === el.form) {
radio.checked = Array.isArray(oldValue)
? oldValue.includes(radio.value)
: oldValue === radio.value;
}
const newValue = Array.isArray(oldValue)
? updateList(oldValue, el.value, el.checked)
: el.checked
? el.value
: undefined;

if (el.name && el.type[0] === "r") {
for (const radio of (
el.getRootNode() as Document | ShadowRoot
).querySelectorAll<HTMLInputElement>(
`[type=radio][name=${CSS.escape(el.name)}]`,
)) {
if (radio.form === el.form) {
radio.checked = Array.isArray(oldValue)
? oldValue.includes(radio.value)
: oldValue === radio.value;
}
} else {
el.checked = !el.checked;
}
} else {
el.checked = !el.checked;
}
checkedValueChange(newValue);
run();

Check warning on line 97 in packages/runtime-tags/src/dom/controllable.ts

View check run for this annotation

Codecov / codecov/patch

packages/runtime-tags/src/dom/controllable.ts#L97

Added line #L97 was not covered by tests
}
});
}
Expand Down Expand Up @@ -151,21 +137,14 @@ export function controllable_input_value_effect(
| undefined
| ((value: unknown) => unknown);
if (valueChange) {
scope[nodeAccessor + AccessorChar.ControlledType] =
ControlledType.Pending;
if (ev) inputType = (ev as InputEvent).inputType;
valueChange(el.value);
const newValue = el.value;
inputType = (ev as InputEvent)?.inputType;
setValueAndUpdateSelection(
el,
scope[nodeAccessor + AccessorChar.ControlledValue],
);
valueChange(newValue);
run();
if (
scope[nodeAccessor + AccessorChar.ControlledType] ===
ControlledType.Pending
) {
setValueAndUpdateSelection(
el,
scope[nodeAccessor + AccessorChar.ControlledValue],
);
}

inputType = "";
}
});
Expand Down Expand Up @@ -210,25 +189,18 @@ export function controllable_select_value_effect(
| undefined
| ((value: unknown) => unknown);
if (valueChange) {
scope[nodeAccessor + AccessorChar.ControlledType] =
ControlledType.Pending;
valueChange(
Array.isArray(scope[nodeAccessor + AccessorChar.ControlledValue])
? Array.from(el.selectedOptions, toValueProp)
: el.value,
const newValue = Array.isArray(
scope[nodeAccessor + AccessorChar.ControlledValue],
)
? Array.from(el.selectedOptions, toValueProp)
: el.value;
setSelectOptions(
el,
scope[nodeAccessor + AccessorChar.ControlledValue],
valueChange,
);
valueChange(newValue);
run();

if (
scope[nodeAccessor + AccessorChar.ControlledType] ===
ControlledType.Pending
) {
setSelectOptions(
el,
scope[nodeAccessor + AccessorChar.ControlledValue],
valueChange,
);
}
}
};

Expand Down Expand Up @@ -312,16 +284,10 @@ export function controllable_detailsOrDialog_open_effect(
nodeAccessor + AccessorChar.ControlledHandler
] as undefined | ((value: unknown) => unknown);
if (openChange && hasChanged()) {
scope[nodeAccessor + AccessorChar.ControlledType] =
ControlledType.Pending;
openChange(el.open);
const newValue = el.open;
el.open = !newValue;
openChange(newValue);

Check warning on line 289 in packages/runtime-tags/src/dom/controllable.ts

View check run for this annotation

Codecov / codecov/patch

packages/runtime-tags/src/dom/controllable.ts#L287-L289

Added lines #L287 - L289 were not covered by tests
run();
if (
scope[nodeAccessor + AccessorChar.ControlledType] ===
ControlledType.Pending
) {
el.open = !el.open;
}
}
},
);
Expand Down

0 comments on commit f99c90f

Please sign in to comment.