Skip to content

Commit

Permalink
refactor: share event handler name logic between translator/runtime
Browse files Browse the repository at this point in the history
  • Loading branch information
DylanPiercey committed Nov 16, 2024
1 parent 7165a78 commit 5b20530
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 38 deletions.
6 changes: 6 additions & 0 deletions .changeset/shy-windows-suffer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@marko/runtime-tags": patch
"@marko/translator-tags": patch
---

Use same event handler name normalization and check between translator and runtime.
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": 17806,
"brotli": 6448
"min": 17850,
"brotli": 6455
}
},
{
Expand Down
13 changes: 9 additions & 4 deletions .sizes/dom.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// size: 17806 (min) 6448 (brotli)
// size: 17850 (min) 6455 (brotli)
var empty = [],
rest = Symbol();
function attrTag(attrs2) {
Expand Down Expand Up @@ -544,6 +544,12 @@ function toDelimitedString(val, delimiter, stringify) {
}
return "";
}
function isEventHandler(name) {
return /^on[A-Z-]/.test(name);
}
function getEventHandlerName(name) {
return "-" === name[2] ? name.slice(3) : name.slice(2).toLowerCase();
}
function normalizeDynamicRenderer(value2) {
if (value2) return value2.renderBody || value2.default || value2;
}
Expand Down Expand Up @@ -836,7 +842,6 @@ var fallback = document.createTextNode(""),
function parseHTML(html2) {
return parser.createContextualFragment(html2);
}
var eventHandlerReg = /^on[A-Z-]/;
function attr(element, name, value2) {
setAttribute(element, name, normalizeAttrValue(value2));
}
Expand Down Expand Up @@ -969,9 +974,9 @@ function attrsInternal(scope, nodeAccessor, nextAttrs) {
case "renderBody":
break;
default:
eventHandlerReg.test(name)
isEventHandler(name)
? ((events ||= scope[nodeAccessor + "~"] = {})[
"-" === name[2] ? name.slice(3) : name.slice(2).toLowerCase()
getEventHandlerName(name)
] = value2)
: skip?.test(name) || attr(el, name, value2);
}
Expand Down
2 changes: 1 addition & 1 deletion .sizes/name-cache.json
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"vars":{"props":{"$empty":"e","$rest":"t","$attrTag":"n","$attrTags":"r","$attrTagIterator":"i","$forIn":"l","$forOf":"o","$forTo":"f","$isScheduled":"u","$port2":"a","$flushAndWaitFrame":"c","$triggerMacroTask":"s","$noop":"d","$createScope":"h","$emptyScope":"g","$getEmptyScope":"p","$destroyScope":"v","$_destroyScope":"b","$onDestroy":"m","$removeAndDestroyScope":"k","$insertBefore":"y","$registeredValues":"w","$Render":"C","$isResuming":"A","$register":"S","$registerBoundSignal":"N","$init":"x","$registerSubscriber":"$","$nodeRef":"M","$MARK":"E","$CLEAN":"I","$DIRTY":"T","$state":"_","$value":"O","$accessorId":"B","$intersection":"V","$defaultGetOwnerScope":"j","$closure":"R","$dynamicClosure":"q","$childClosures":"D","$dynamicSubscribers":"P","$setTagVar":"W","$tagVarSignal":"L","$renderBodyClosures":"z","$tagIdsByGlobal":"F","$nextTagId":"U","$inChild":"G","$intersections":"J","$effect":"X","$pendingSignals":"Z","$pendingEffects":"H","$rendering":"K","$queueEffect":"Q","$run":"Y","$prepareEffects":"ee","$runEffects":"te","$runSignals":"ne","$resetAbortSignal":"re","$getAbortSignal":"ie","$stringifyClassObject":"le","$NON_DIMENSIONAL":"oe","$stringifyStyleObject":"fe","$toDelimitedString":"ue","$normalizeDynamicRenderer":"ae","$elementHandlersByEvent":"ce","$defaultDelegator":"se","$on":"de","$createDelegator":"he","$handleDelegated":"ge","$stripSpacesAndPunctuation":"pe","$controllable_input_checked":"ve","$controllable_input_checked_effect":"be","$controllable_input_checkedValue":"me","$controllable_input_checkedValue_effect":"ke","$controllable_input_value":"ye","$controllable_input_value_effect":"we","$controllable_select_value":"Ce","$controllable_select_value_effect":"Ae","$setSelectOptions":"Se","$controllable_detailsOrDialog_open":"Ne","$controllable_detailsOrDialog_open_effect":"xe","$inputType":"$e","$setValueAndUpdateSelection":"Me","$setCheckboxValue":"Ee","$delegateFormControl":"Ie","$formChangeHandlers":"Te","$syncControllable":"_e","$onFormChange":"Oe","$onFormReset":"Be","$hasValueChanged":"Ve","$hasCheckboxChanged":"je","$hasSelectChanged":"Re","$hasFormElementChanged":"qe","$normalizeStrProp":"De","$normalizeBoolProp":"Pe","$toValueProp":"We","$fallback":"Le","$parser":"ze","$parseHTML":"Fe","$eventHandlerReg":"Ue","$attr":"Ge","$setAttribute":"Je","$classAttr":"Xe","$styleAttr":"Ze","$data":"He","$attrs":"Ke","$hasAttrAlias":"Qe","$partialAttrs":"Ye","$attrsInternal":"et","$attrsEvents":"tt","$html":"nt","$props":"rt","$normalizeAttrValue":"it","$lifecycle":"lt","$walker":"ot","$trimWalkString":"ft","$walk":"ut","$walkInternal":"at","$createScopeWithRenderer":"ct","$createScopeWithTagNameOrRenderer":"st","$initRenderer":"dt","$dynamicTagAttrs":"ht","$createRendererWithOwner":"gt","$createRenderer":"pt","$_clone":"vt","$conditional":"bt","$inConditionalScope":"mt","$conditionalOnlyChild":"kt","$setConditionalRendererOnlyChild":"yt","$emptyMarkerMap":"wt","$emptyMarkerArray":"Ct","$emptyMap":"At","$emptyArray":"St","$loopOf":"Nt","$loopIn":"xt","$loopTo":"$t","$loop":"Mt","$inLoopScope":"Et","$bySecondArg":"It","$byFirstArg":"Tt","$isDifferentRenderer":"_t","$classIdToScope":"Ot","$compat":"Bt","$createTemplate":"Vt","$mount":"jt","$parseHTMLOrSingleNode":"Wt","$marker":"qt","$_clickCount_effect":"Dt","$_clickCount":"Xt","$_setup_":"Zt","$_expr_comment_id$ifBody":"ss","$_id$ifBody":"as","$_comment$ifBody":"ns","$_ifBody":"ts","$_expr_input_i$forBody":"os","$_if$forBody":"cs","$_open$forBody_effect":"is","$_open$forBody":"ms","$_id$forBody":"ls","$_i$forBody":"us","$_comment$forBody":"es","$_params_2$forBody":"ds","$_input$forBody":"rs","$_for":"bs","$_input_$1":"fs","$_input_":"ps","$_params__":"vs"}}}
{"vars":{"props":{"$empty":"e","$rest":"t","$attrTag":"n","$attrTags":"r","$attrTagIterator":"i","$forIn":"l","$forOf":"o","$forTo":"f","$isScheduled":"u","$port2":"a","$flushAndWaitFrame":"c","$triggerMacroTask":"s","$noop":"d","$createScope":"h","$emptyScope":"g","$getEmptyScope":"p","$destroyScope":"v","$_destroyScope":"b","$onDestroy":"m","$removeAndDestroyScope":"k","$insertBefore":"y","$registeredValues":"w","$Render":"C","$isResuming":"A","$register":"S","$registerBoundSignal":"N","$init":"x","$registerSubscriber":"$","$nodeRef":"M","$MARK":"E","$CLEAN":"I","$DIRTY":"T","$state":"_","$value":"O","$accessorId":"B","$intersection":"V","$defaultGetOwnerScope":"j","$closure":"R","$dynamicClosure":"q","$childClosures":"D","$dynamicSubscribers":"P","$setTagVar":"W","$tagVarSignal":"L","$renderBodyClosures":"z","$tagIdsByGlobal":"F","$nextTagId":"U","$inChild":"G","$intersections":"J","$effect":"X","$pendingSignals":"Z","$pendingEffects":"H","$rendering":"K","$queueEffect":"Q","$run":"Y","$prepareEffects":"ee","$runEffects":"te","$runSignals":"ne","$resetAbortSignal":"re","$getAbortSignal":"ie","$stringifyClassObject":"le","$NON_DIMENSIONAL":"oe","$stringifyStyleObject":"fe","$toDelimitedString":"ue","$normalizeDynamicRenderer":"ae","$elementHandlersByEvent":"ce","$defaultDelegator":"se","$on":"de","$createDelegator":"he","$handleDelegated":"ge","$stripSpacesAndPunctuation":"pe","$controllable_input_checked":"ve","$controllable_input_checked_effect":"be","$controllable_input_checkedValue":"me","$controllable_input_checkedValue_effect":"ke","$controllable_input_value":"ye","$controllable_input_value_effect":"we","$controllable_select_value":"Ce","$controllable_select_value_effect":"Ae","$setSelectOptions":"Se","$controllable_detailsOrDialog_open":"Ne","$controllable_detailsOrDialog_open_effect":"xe","$inputType":"$e","$setValueAndUpdateSelection":"Me","$setCheckboxValue":"Ee","$delegateFormControl":"Ie","$formChangeHandlers":"Te","$syncControllable":"_e","$onFormChange":"Oe","$onFormReset":"Be","$hasValueChanged":"Ve","$hasCheckboxChanged":"je","$hasSelectChanged":"Re","$hasFormElementChanged":"qe","$normalizeStrProp":"De","$normalizeBoolProp":"Pe","$toValueProp":"We","$fallback":"Le","$parser":"ze","$parseHTML":"Fe","$eventHandlerReg":"Ue","$attr":"Ge","$setAttribute":"Je","$classAttr":"Xe","$styleAttr":"Ze","$data":"He","$attrs":"Ke","$hasAttrAlias":"Qe","$partialAttrs":"Ye","$attrsInternal":"et","$attrsEvents":"tt","$html":"nt","$props":"rt","$normalizeAttrValue":"it","$lifecycle":"lt","$walker":"ot","$trimWalkString":"ft","$walk":"ut","$walkInternal":"at","$createScopeWithRenderer":"ct","$createScopeWithTagNameOrRenderer":"st","$initRenderer":"dt","$dynamicTagAttrs":"ht","$createRendererWithOwner":"gt","$createRenderer":"pt","$_clone":"vt","$conditional":"bt","$inConditionalScope":"mt","$conditionalOnlyChild":"kt","$setConditionalRendererOnlyChild":"yt","$emptyMarkerMap":"wt","$emptyMarkerArray":"Ct","$emptyMap":"At","$emptyArray":"St","$loopOf":"Nt","$loopIn":"xt","$loopTo":"$t","$loop":"Mt","$inLoopScope":"Et","$bySecondArg":"It","$byFirstArg":"Tt","$isDifferentRenderer":"_t","$classIdToScope":"Ot","$compat":"Bt","$createTemplate":"Vt","$mount":"jt","$parseHTMLOrSingleNode":"Wt","$marker":"qt","$_clickCount_effect":"Dt","$_clickCount":"Xt","$_setup_":"Zt","$_expr_comment_id$ifBody":"ss","$_id$ifBody":"as","$_comment$ifBody":"ns","$_ifBody":"ts","$_expr_input_i$forBody":"os","$_if$forBody":"cs","$_open$forBody_effect":"is","$_open$forBody":"ms","$_id$forBody":"ls","$_i$forBody":"us","$_comment$forBody":"es","$_params_2$forBody":"ds","$_input$forBody":"rs","$_for":"bs","$_input_$1":"fs","$_input_":"ps","$_params__":"vs","$isEventHandler":"Rt","$getEventHandlerName":"Pt"}}}
7 changes: 7 additions & 0 deletions packages/runtime-tags/src/common/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,13 @@ function toDelimitedString(
return "";
}

export function isEventHandler(name: string): name is `on${string}` {
return /^on[A-Z-]/.test(name);
}
export function getEventHandlerName(name: `on${string}`) {
return name[2] === "-" ? name.slice(3) : name.slice(2).toLowerCase();
}

export function isVoid(value: unknown) {
return value == null || value === false;
}
Expand Down
16 changes: 10 additions & 6 deletions packages/runtime-tags/src/dom/dom.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { classValue, styleValue } from "../common/helpers";
import {
classValue,
getEventHandlerName,
isEventHandler,
styleValue,
} from "../common/helpers";
import {
type Accessor,
AccessorChar,
Expand All @@ -23,8 +28,6 @@ import {
import { on } from "./event";
import { parseHTML } from "./parse-html";

const eventHandlerReg = /^on[A-Z-]/;

export function isDocumentFragment(node: Node): node is DocumentFragment {
return node.nodeType === NodeType.DocumentFragment;
}
Expand Down Expand Up @@ -203,14 +206,15 @@ function attrsInternal(
break;
case "renderBody":
break;
default:
if (eventHandlerReg.test(name)) {
default: {
if (isEventHandler(name)) {
(events ||= scope[nodeAccessor + AccessorChar.EventAttributes] = {})[
name[2] === "-" ? name.slice(3) : name.slice(2).toLowerCase()
getEventHandlerName(name)

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

View check run for this annotation

Codecov / codecov/patch

packages/runtime-tags/src/dom/dom.ts#L212

Added line #L212 was not covered by tests
] = value;
} else if (!skip?.test(name)) {
attr(el, name, value);
}
}

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

View check run for this annotation

Codecov / codecov/patch

packages/runtime-tags/src/dom/dom.ts#L217

Added line #L217 was not covered by tests
}
}
}
Expand Down
33 changes: 18 additions & 15 deletions packages/runtime-tags/src/html/attrs.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { classValue, isVoid, styleValue } from "../common/helpers";
import {
classValue,
getEventHandlerName,
isEventHandler,
isVoid,
styleValue,
} from "../common/helpers";
import { type Accessor, AccessorChar, ControlledType } from "../common/types";
import { escapeTextAreaValue } from "./content";
import { ensureScopeWithId, getChunk, withContext } from "./writer";
Expand Down Expand Up @@ -35,7 +41,7 @@ export function controllable_select_value(
renderBody?: () => void,
) {
if (valueChange) {
const scope = ensureScopeWithId(scopeId!);
const scope = ensureScopeWithId(scopeId);
scope[nodeAccessor + AccessorChar.ControlledValue] = value;
scope[nodeAccessor + AccessorChar.ControlledHandler] = valueChange;
scope[nodeAccessor + AccessorChar.ControlledType] =
Expand All @@ -54,7 +60,7 @@ export function controllable_textarea_value(
valueChange: unknown,
) {
if (valueChange) {
const scope = ensureScopeWithId(scopeId!);
const scope = ensureScopeWithId(scopeId);
scope[nodeAccessor + AccessorChar.ControlledHandler] = valueChange;
scope[nodeAccessor + AccessorChar.ControlledType] =
ControlledType.InputValue;
Expand All @@ -70,7 +76,7 @@ export function controllable_input_value(
valueChange: unknown,
) {
if (valueChange) {
const scope = ensureScopeWithId(scopeId!);
const scope = ensureScopeWithId(scopeId);
scope[nodeAccessor + AccessorChar.ControlledHandler] = valueChange;
scope[nodeAccessor + AccessorChar.ControlledType] =
ControlledType.InputValue;
Expand All @@ -85,7 +91,7 @@ export function controllable_input_checked(
checkedChange: unknown,
) {
if (checkedChange) {
const scope = ensureScopeWithId(scopeId!);
const scope = ensureScopeWithId(scopeId);
// scope[nodeAccessor + AccessorChar.ControlledValue] = checked;
scope[nodeAccessor + AccessorChar.ControlledHandler] = checkedChange;
scope[nodeAccessor + AccessorChar.ControlledType] =
Expand All @@ -104,7 +110,7 @@ export function controllable_input_checkedValue(
const multiple = Array.isArray(checkedValue);
const valueAttr = attr("value", value);
if (checkedValueChange) {
const scope = ensureScopeWithId(scopeId!);
const scope = ensureScopeWithId(scopeId);
scope[nodeAccessor + AccessorChar.ControlledHandler] = checkedValueChange;
scope[nodeAccessor + AccessorChar.ControlledType] =
ControlledType.InputCheckedValue;
Expand All @@ -125,7 +131,7 @@ export function controllable_detailsOrDialog_open(
openChange: unknown,
) {
if (openChange) {
const scope = ensureScopeWithId(scopeId!);
const scope = ensureScopeWithId(scopeId);

Check warning on line 134 in packages/runtime-tags/src/html/attrs.ts

View check run for this annotation

Codecov / codecov/patch

packages/runtime-tags/src/html/attrs.ts#L134

Added line #L134 was not covered by tests
scope[nodeAccessor + AccessorChar.ControlledValue] = open;
scope[nodeAccessor + AccessorChar.ControlledHandler] = openChange;
scope[nodeAccessor + AccessorChar.ControlledType] =
Expand All @@ -146,7 +152,6 @@ export function attrs(
) {
let result = "";
let skip = /[\s/>"'=]/;
let scope: Record<string, unknown> | undefined;
let events: Record<string, unknown> | undefined;
switch (tagName) {
case "input":
Expand Down Expand Up @@ -218,13 +223,11 @@ export function attrs(
break;
default:
if (!isVoid(val)) {
if (/^on[A-Z-]/.test(name)) {
events ||= (scope ??= ensureScopeWithId(scopeId!))[
(nodeAccessor + AccessorChar.EventAttributes) as any
] ||= {} as any;
events![
name[2] === "-" ? name.slice(3) : name.slice(2).toLowerCase()
] = val;
if (isEventHandler(name)) {
(events ||= ensureScopeWithId(scopeId)[
nodeAccessor + AccessorChar.EventAttributes
] =
{})[getEventHandlerName(name)] = val;
} else if (!skip.test(name)) {
result += nonVoidAttr(name, val);
}
Expand Down
14 changes: 4 additions & 10 deletions packages/translator-tags/src/visitors/tag/native-tag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ import {
getTagDef,
} from "@marko/babel-utils";
import { types as t } from "@marko/compiler";
import {
getEventHandlerName,
isEventHandler,
} from "@marko/runtime-tags/common/helpers";
import { WalkCode } from "@marko/runtime-tags/common/types";

import evaluate from "../../util/evaluate";
Expand Down Expand Up @@ -802,20 +806,10 @@ function getUsedAttrs(tagName: string, tag: t.MarkoTag) {
};
}

function isEventHandler(propName: string) {
return /^on[A-Z-]/.test(propName);
}

function isChangeHandler(propName: string) {
return /^(?:value|checked(?:Values?)?|open)Change/.test(propName);
}

function getEventHandlerName(propName: string) {
return propName.charAt(2) === "-"
? propName.slice(3)
: propName.charAt(2).toLowerCase() + propName.slice(3);
}

function attrToObjectProperty(attr: t.MarkoAttribute) {
return t.objectProperty(toPropertyName(attr.name), attr.value);
}
Expand Down

0 comments on commit 5b20530

Please sign in to comment.