diff --git a/.changeset/lowercase-on-is-attribute.md b/.changeset/lowercase-on-is-attribute.md new file mode 100644 index 000000000..0516564df --- /dev/null +++ b/.changeset/lowercase-on-is-attribute.md @@ -0,0 +1,16 @@ +--- +"@solidjs/babel-plugin": patch +"@solidjs/compiler": patch +"@solidjs/web": patch +"@solidjs/html": patch +"@solidjs/h": patch +"@solidjs/signals": patch +--- + +**Breaking:** only `on` followed by an uppercase letter (`onClick`, `onPointerDown`) is an event handler. Lowercase `on*` names (`onclick`, `onmouseover`) are plain attributes everywhere: + +- Both compilers compile `onclick={expr}` like any other attribute (`setAttribute`, reactive when `expr` is dynamic) instead of binding a delegated or native event, and SSR renders it as an escaped attribute instead of dropping it. A leftover 1.x `on:click={fn}` is likewise a plain namespaced attribute (it previously compiled to `addEventListener(":click", fn)`). +- `@solidjs/web` `spread`/`assign` set lowercase `on*` keys as attributes, the server spread walk renders them, and `useHead` applies lowercase `on*` attributes (camelCase handler names stay skipped). `ssrAttribute` escapes a function value instead of interpolating its source raw. +- `@solidjs/html` and `@solidjs/h` elements wrap a function passed to a lowercase `on*` in a getter like any other attribute; only `onXxx` and `ref` are exempt. +- `@solidjs/html` components follow `@solidjs/h`'s rule: every zero-argument function prop, `onXxx` handlers and `ref` included, is a getter, so a component handler must declare its event argument (`onClick=${e => …}`). +- New dev-only check `LOWERCASE_EVENT_ATTRIBUTE` (added to the `DiagnosticCode` union): warns once per attribute name when a function is set on a lowercase `on*` (or `on:`) attribute, naming the camelCase handler to use. diff --git a/documentation/solid-2.0/07-dom.md b/documentation/solid-2.0/07-dom.md index 177f00c0d..34d471747 100644 --- a/documentation/solid-2.0/07-dom.md +++ b/documentation/solid-2.0/07-dom.md @@ -18,7 +18,7 @@ DOM behavior in Solid 2.0 follows HTML standards by default: attributes over pro - **Attributes over properties:** Prefer setting attributes rather than properties in almost all cases. Aligns with web components and SSR. - **Lowercasing:** Use HTML lowercase for built-in attribute names (no camelCase for attributes). Exceptions: - - **Event handlers** remain camelCase (e.g. `onClick`) to keep the `on` modifier clear. + - **Event handlers** remain camelCase (e.g. `onClick`) to keep the `on` modifier clear. Only `on` followed by an uppercase letter is an event handler. Lowercase `on*` names (`onclick`, `onmouseover`) are plain attributes like any other — `onclick="track()"` is the HTML inline-handler attribute, and `onclick={expr}` sets that attribute (reactively when `expr` is dynamic) instead of binding an event. The JSX types do not declare lowercase `on*` names, and in development passing a function to one warns (`LOWERCASE_EVENT_ATTRIBUTE`). The same rule applies to `spread`, `@solidjs/html` and `@solidjs/h` elements, and SSR, which renders lowercase `on*` attributes. - **Default to attributes** But attributes such as `input.value`, `input.defaultValue`, `input.checked`, `input.defaultChecked`, `select.value`, `option.value`, `option.selected`, `option.defaultSelected`, `textarea.value`, `textarea.defaultValue`, `video.muted`, `video.defaultMuted`, `audio.muted`, `audio.defaultMuted` continue to be handled as props where that avoids confusion. Unfortunately, this leads to all form fields be special cased. For example: `` either can be dynamic or static, and in the absense of `defaultValue`, then, `value` is SSRed. - **Namespaces:** `attr:`, `bool:`, and `on:` namespaces are removed; the single standard behavior makes the model consistent. The compiler also no longer gives special handling to previously tolerated `class:` / `style:` namespace syntax. Use `class` object/array values for class toggles, `style` object values for style properties, and standard attributes/properties for everything else. - **XML Namespaces:** `svg` and `math` work as expected, however when using XML partials, an `xmlns` attribute is required for the browser to create the elements with the correct namespace. Solid adds these automatically to the tags that can recognize as SVG/MathML. For example an `a` tag returned from a partial to be used in XML need `xmlns` added by the user. diff --git a/documentation/solid-2.0/08-dev-diagnostics.md b/documentation/solid-2.0/08-dev-diagnostics.md index 632778f58..f8fc04f2d 100644 --- a/documentation/solid-2.0/08-dev-diagnostics.md +++ b/documentation/solid-2.0/08-dev-diagnostics.md @@ -699,6 +699,12 @@ Check (`warn`, dev only; kind `render`; server render and client hydrate; once p Unscoped allocation alone is not the finding: a function hole with nothing scoped after it in its template lands on the same ids on both sides and stays silent. (An `` zero-arity `fallback={() => }` thunk used to be the common instance — handed back unresolved and built by the consuming hole; `` now calls a function-valued fallback inside its own scope whatever its arity, so that shape never reaches a hole.) Each side reports the permutation it can see. The server records the counter's next id when the hole was registered (`data.registered`, argument evaluation — where the client builds it) and around its evaluation in the walk (`data.before` → `data.after`), and reports when the hole allocated at a shifted position; `data.hole` is the hole's position in its template. The client always builds in place, so it reports when the content built inside an unscoped function hole moved the counter **and** missed a server-rendered key (`Hydration key miss …` is the symptom; this is the cause). `data.name` is the function (when it has one). Scoped holes, memo and component accessors, `children()`, `` rows and the runtime's own children inserts (`spread`, `Portal`) never raise it. Fix: call the function at the hole (`{renderHead()}` — a call hole is scoped on both sides) or pass the built value. +#### `LOWERCASE_EVENT_ATTRIBUTE` + +**Message:** "[LOWERCASE_EVENT_ATTRIBUTE] `onclick` received a function, but `onclick` is an attribute in Solid 2.0, not an event handler: the function was set as the attribute's text. Use `onClick` for event handlers." + +Check (`warn`, dev only; kind `render`; client, once per attribute name). A function reached an attribute whose name starts with `on` but is not an event handler. Only `on` followed by an uppercase letter (`onClick`) binds an event in 2.0; a lowercase `onclick` — or a leftover 1.x `on:click` — is a plain attribute, so the function is stringified into it and never runs as a handler. Raised from the runtime's attribute write, so it covers compiled attributes, `spread`/`assign`, and hydration alike. `data.name` is the attribute, `data.handler` the camelCase name to use, `data.tag` the element. The JSX types do not declare lowercase `on*` names, so this is reached from JavaScript, through a cast, or from untyped props. Fix: rename to the camelCase handler (`onclick={save}` → `onClick={save}`); keep the lowercase name only for a string inline-handler attribute. + #### `BINDING_SLOT_POSITION` **Messages:** @@ -980,6 +986,7 @@ The runtime derives a request's trace itself in every tier — the W3C `tracepar | `HEAD_TAG_INVALID` | warn | head | `useHead` registration the render could not honor; `data.reason` names the rule (dev) | | `UNRECOGNIZED_INSERT_VALUE` | warn | render | Value at an insert position the renderer cannot render; skipped (dev; server and client) | | `UNSCOPED_HOLE_ALLOCATED_IDS` | warn | render | Unscoped hole was handed a function whose content took ids at a position the other side does not share; keys permute (dev) | +| `LOWERCASE_EVENT_ATTRIBUTE` | warn | render | A function was set on a lowercase `on*` (or `on:`) attribute, which is not an event handler in 2.0; use `onXxx` (dev; client) | | `BINDING_SLOT_POSITION` | warn/err | ssr/render | Binding-slot value where it cannot bind; `data.reason` names the position (spread is an error and throws), client reasons `orphan`/`fill-shape` | ## Run attribution — "why did this run" diff --git a/documentation/solid-2.0/MIGRATION.md b/documentation/solid-2.0/MIGRATION.md index a6671629f..3d94a7489 100644 --- a/documentation/solid-2.0/MIGRATION.md +++ b/documentation/solid-2.0/MIGRATION.md @@ -587,7 +587,18 @@ Solid 2.0 aims to be more “what you write is what the platform sees”: ``` -`on:` and `oncapture:` are removed. Keep using camelCase event handlers like `onClick` for Solid-managed events. For native listener options, use a ref callback: +Event handlers are camelCase only: `on` followed by an uppercase letter (`onClick`, `onPointerDown`). **Lowercase `on*` names are attributes in 2.0.** In 1.x ` + - - - - + + + + ); + +const lowercaseAttributes = ( +
+ + + + + +
+); diff --git a/packages/babel-plugin/test/__dom_fixtures__/eventExpressions/output.js b/packages/babel-plugin/test/__dom_fixtures__/eventExpressions/output.js index 1f0f7856d..7291e7a0c 100644 --- a/packages/babel-plugin/test/__dom_fixtures__/eventExpressions/output.js +++ b/packages/babel-plugin/test/__dom_fixtures__/eventExpressions/output.js @@ -1,9 +1,15 @@ import { template as _$template } from "r-dom"; import { delegateEvents as _$delegateEvents } from "r-dom"; +import { effect as _$effect } from "r-dom"; +import { spread as _$spread } from "r-dom"; +import { setAttribute as _$setAttribute } from "r-dom"; import { addEvent as _$addEvent } from "r-dom"; var _tmpl$ = /*#__PURE__*/ _$template( - `
+ - +
); + +const lowercaseAttributes = ( +
+ + + + + +
+); diff --git a/packages/babel-plugin/test/__dom_hydratable_fixtures__/eventExpressions/output.js b/packages/babel-plugin/test/__dom_hydratable_fixtures__/eventExpressions/output.js index d186a9df4..671767331 100644 --- a/packages/babel-plugin/test/__dom_hydratable_fixtures__/eventExpressions/output.js +++ b/packages/babel-plugin/test/__dom_hydratable_fixtures__/eventExpressions/output.js @@ -1,10 +1,16 @@ import { template as _$template } from "r-dom"; import { delegateEvents as _$delegateEvents } from "r-dom"; +import { effect as _$effect } from "r-dom"; +import { spread as _$spread } from "r-dom"; +import { setAttribute as _$setAttribute } from "r-dom"; import { getNextElement as _$getNextElement } from "r-dom"; import { runHydrationEvents as _$runHydrationEvents } from "r-dom"; var _tmpl$ = /*#__PURE__*/ _$template( - `
+ - - - - + + + +
); + +const lowercaseAttributes = ( +
+ + + + + +
+); diff --git a/packages/babel-plugin/test/__dynamic_fixtures__/eventExpressions/output.js b/packages/babel-plugin/test/__dynamic_fixtures__/eventExpressions/output.js index 099dde27e..09d4048dd 100644 --- a/packages/babel-plugin/test/__dynamic_fixtures__/eventExpressions/output.js +++ b/packages/babel-plugin/test/__dynamic_fixtures__/eventExpressions/output.js @@ -1,9 +1,15 @@ import { template as _$template } from "r-dom"; import { delegateEvents as _$delegateEvents } from "r-dom"; +import { effect as _$effect } from "r-custom"; +import { spread as _$spread } from "r-dom"; +import { setAttribute as _$setAttribute } from "r-dom"; import { addEvent as _$addEvent } from "r-dom"; var _tmpl$ = /*#__PURE__*/ _$template( - `
+ - +
); + +const lowercaseAttributes = ( +
+ + + + + +
+); diff --git a/packages/babel-plugin/test/__ssr_hydratable_fixtures__/eventExpressions/output.js b/packages/babel-plugin/test/__ssr_hydratable_fixtures__/eventExpressions/output.js index 55048275e..26a860b66 100644 --- a/packages/babel-plugin/test/__ssr_hydratable_fixtures__/eventExpressions/output.js +++ b/packages/babel-plugin/test/__ssr_hydratable_fixtures__/eventExpressions/output.js @@ -1,12 +1,43 @@ +import { ssrElement as _$ssrElement } from "r-server"; +import { ssrElementAttribute as _$ssrElementAttribute } from "r-server"; +import { ssrAttribute as _$ssrAttribute } from "r-server"; +import { escape as _$escape } from "r-server"; import { ssr as _$ssr } from "r-server"; import { ssrHydrationKey as _$ssrHydrationKey } from "r-server"; var _tmpl$ = [ - "' -]; + "' + ], + _tmpl$2 = [ + "Identifier AttributeDynamic AttributeFunction Attribute", + "" + ]; +var _sk$ = k => k === "onclick"; function hoistedCustomEvent1() { console.log("hoisted"); } const hoistedcustomevent2 = () => console.log("hoisted"); var _v$ = _$ssrHydrationKey(); const template = _$ssr(_tmpl$, _v$); +var _v$2 = _$ssrHydrationKey(), + _v$3 = () => _$ssrAttribute("onmouseover", _$escape(state.code, true)), + _v$4 = _$ssrElement( + "button", + rest, + () => "Spread Attribute", + false, + _sk$, + () => _$ssrElementAttribute("onclick", code) + ); +const lowercaseAttributes = _$ssr( + _tmpl$2, + _v$2, + _$ssrAttribute("onclick", _$escape(code, true)), + _v$3, + _$ssrAttribute("onclick", () => _$escape(console.log("not a handler"), true)), + _v$4 +); diff --git a/packages/compiler/__tests__/fixtures/dom-hydratable/eventExpressions/output.js b/packages/compiler/__tests__/fixtures/dom-hydratable/eventExpressions/output.js index 087b14161..4ea336e6a 100644 --- a/packages/compiler/__tests__/fixtures/dom-hydratable/eventExpressions/output.js +++ b/packages/compiler/__tests__/fixtures/dom-hydratable/eventExpressions/output.js @@ -1,8 +1,12 @@ import { template as _$template } from "r-dom"; import { getNextElement as _$getNextElement } from "r-dom"; +import { spread as _$spread } from "r-dom"; +import { effect as _$effect } from "r-dom"; +import { setAttribute as _$setAttribute } from "r-dom"; import { delegateEvents as _$delegateEvents } from "r-dom"; import { runHydrationEvents as _$runHydrationEvents } from "r-dom"; var _tmpl$ = /* @__PURE__ */ _$template(`
"]; +var _tmpl$2 = [ + "Identifier AttributeDynamic AttributeFunction Attribute", + "" +]; +var _sk$ = (k) => k === "onclick"; function hoistedCustomEvent1() { console.log("hoisted"); } const hoistedcustomevent2 = () => console.log("hoisted"); var _v$ = _$ssrHydrationKey(); const template = _$ssr(_tmpl$, _v$); +var _v$2 = _$ssrHydrationKey(), _v$3 = () => { + return _$ssrAttribute("onmouseover", _$escape(state.code, true)); +}, _v$4 = _$ssrElement("button", rest, () => { + return "Spread Attribute"; +}, false, _sk$, () => _$ssrElementAttribute("onclick", code)); +const lowercaseAttributes = _$ssr(_tmpl$2, _v$2, _$ssrAttribute("onclick", _$escape(code, true)), _v$3, _$ssrAttribute("onclick", () => _$escape(console.log("not a handler"), true)), _v$4); diff --git a/packages/compiler/__tests__/parity-probes.test.js b/packages/compiler/__tests__/parity-probes.test.js index 9ea9de017..5fd816e71 100644 --- a/packages/compiler/__tests__/parity-probes.test.js +++ b/packages/compiler/__tests__/parity-probes.test.js @@ -306,11 +306,16 @@ const a = {x()}; "bound event array": ` const a = ; `, - "on namespace event": ` + "removed on: namespaces are attributes": ` const a =
; `, - "lowercase and camel events": ` + "lowercase on* attributes beside camel events": ` const a =
; +`, + "lowercase on* function, dynamic and spread attributes": ` +const a =
go()} onmouseover={state.code} oninput="run()" />; +const b =
; +const c =
; `, "class and style namespaces": ` const a =
; diff --git a/packages/compiler/__tests__/transform.test.js b/packages/compiler/__tests__/transform.test.js index 1212487cb..411d514ed 100644 --- a/packages/compiler/__tests__/transform.test.js +++ b/packages/compiler/__tests__/transform.test.js @@ -909,18 +909,42 @@ describe("@solidjs/compiler transform", () => { expect(result.code).not.toContain("_$delegateEvents"); }); - it("treats namespaced event attributes like Babel after the event update", () => { - // The `on:`/`oncapture:` namespaces were removed on this branch; Babel's - // `key.startsWith("on")` branch now sees the raw namespaced key. + it("treats removed `on:` namespaced names as plain attributes", () => { + // Only `on` + an uppercase letter is an event handler; `on:click` is an + // unknown namespace like any other. const result = transform(" + + +
` as HTMLElement; + return dispose; + }); + const [first, second, third] = el.querySelectorAll("button"); + expect(first.getAttribute("onclick")).toBe("run()"); + expect(second.getAttribute("onmouseover")).toBe("first()"); + expect(third.getAttribute("onclick")).toBe("expr()"); + expect((third as any)._$$click).toBeUndefined(); + setCode("second()"); + flush(); + expect(second.getAttribute("onmouseover")).toBe("second()"); + dispose(); + }); + it("integrates ref listeners and delegated events", () => { const exec = { first: false, delegated: false, second: false }; const container = document.createElement("div"); @@ -430,10 +452,10 @@ describe("Tagged JSX Integration Tests", () => { dispose(); })); - it("wraps zero-arg functions in getters for non-handler `on*` props (#3728)", () => { + it("wraps every zero-arg function prop on a component in a getter (#3728)", () => { const [value, setValue] = createSignal(1); - const handler = () => "handled"; - const ref = () => {}; + const handler = (e: Event) => e; + const ref = (el: unknown) => el; let props!: any; const Probe = (p: any) => { props = p; @@ -444,8 +466,10 @@ describe("Tagged JSX Integration Tests", () => { on=${() => value()} only=${() => value() * 2} once=${() => value() * 3} - onClick=${handler} - ref=${ref} + onClick=${() => value() * 4} + onInput=${handler} + ref=${() => value() * 5} + refWithArg=${ref} />`; return d; }); @@ -453,14 +477,18 @@ describe("Tagged JSX Integration Tests", () => { expect(props.on).toBe(1); expect(props.only).toBe(2); expect(props.once).toBe(3); - expect(props.onClick).toBe(handler); - expect(props.ref).toBe(ref); + expect(props.onClick).toBe(4); + expect(props.onInput).toBe(handler); + expect(props.ref).toBe(5); + expect(props.refWithArg).toBe(ref); setValue(2); flush(); expect(props.on).toBe(2); expect(props.only).toBe(4); expect(props.once).toBe(6); + expect(props.onClick).toBe(8); + expect(props.ref).toBe(10); dispose(); }); diff --git a/packages/signals/src/core/dev.ts b/packages/signals/src/core/dev.ts index c3de22c72..60d588726 100644 --- a/packages/signals/src/core/dev.ts +++ b/packages/signals/src/core/dev.ts @@ -103,6 +103,7 @@ export type DiagnosticCode = | "HEAD_TAG_INVALID" | "UNRECOGNIZED_INSERT_VALUE" | "UNSCOPED_HOLE_ALLOCATED_IDS" + | "LOWERCASE_EVENT_ATTRIBUTE" | "BINDING_SLOT_POSITION" | "FRAME_MARKER_CORRUPTED" | "DYNAMIC_ASYNC_COMPONENT"; @@ -122,7 +123,7 @@ export type DiagnosticKind = | "ssr" /** The response head: `` tags, preload descriptors, HTTP headers. */ | "head" - /** The renderer's insert positions, on either platform: a value it has no rendering for. */ + /** The renderer's insert and attribute positions: a value it has no rendering for, or one it renders other than intended. */ | "render"; /** First warning when a change reaches (or a pass tracks) this many edges. */ diff --git a/packages/solid/skills/reactivity-diagnostics/SKILL.md b/packages/solid/skills/reactivity-diagnostics/SKILL.md index d9b2d0caa..ed59520c8 100644 --- a/packages/solid/skills/reactivity-diagnostics/SKILL.md +++ b/packages/solid/skills/reactivity-diagnostics/SKILL.md @@ -843,6 +843,16 @@ JavaScript or through a cast. Fix: call the function at the hole (`{renderHead()}` — a call hole is scoped on both sides) or assign the built value first and insert that. +### LOWERCASE_EVENT_ATTRIBUTE + +A function was set on an attribute whose name starts with `on` but is not an +event handler (`data.name`): a lowercase `onclick`, or a leftover 1.x +`on:click`. In 2.0 only `on` + an uppercase letter binds an event; anything +else is a plain attribute, so the function was stringified into it and never +runs. Once per attribute name. Fix: use the camelCase handler +(`data.handler` — `onclick={save}` → `onClick={save}`); keep a lowercase name +only for a string inline-handler attribute. + ### BINDING_SLOT_POSITION A binding slot's property (`const row = props.row(args); row.done`) diff --git a/packages/web/frames/src/client.ts b/packages/web/frames/src/client.ts index 90a6b8a02..b29575cb4 100644 --- a/packages/web/frames/src/client.ts +++ b/packages/web/frames/src/client.ts @@ -468,14 +468,16 @@ function bindDataOccurrence( let st = state.get(element); if (!st) state.set(element, (st = { prev: {}, handlers: {}, ref: undefined, refId: "" })); // Handler positions: the marker's event name (`onClick` compiled to - // `click`) as the prop `assign` binds. The server merges duplicate - // handlers last-wins, so a position names one key; given more, the last. - // Several keys at a ref position all fire, in marker order. + // `click`) as the prop `assign` binds. The prop must be `on` + an + // uppercase letter (`onClick`) — a lowercase `onclick` is an attribute + // to `assign`. The server merges duplicate handlers last-wins, so a + // position names one key; given more, the last. Several keys at a ref + // position all fire, in marker order. const handlers: Record = {}; const refKeys: string[] = []; for (const { pos, key } of positions) { if (pos === "ref") refKeys.push(key); - else if (pos.startsWith("on:")) handlers["on" + pos.slice(3)] = key; + else if (pos.startsWith("on:")) handlers["on" + pos[3].toUpperCase() + pos.slice(4)] = key; } let owners = handlerOwners.get(element); if (!owners) handlerOwners.set(element, (owners = {})); diff --git a/packages/web/src/client.ts b/packages/web/src/client.ts index 6d65010f0..490932549 100644 --- a/packages/web/src/client.ts +++ b/packages/web/src/client.ts @@ -186,7 +186,7 @@ import { qualifierValue, STYLESHEET_FETCH_META } from "./head.js"; -import { devCheck, unscopedHoleAllocatedIds } from "./diagnostics.js"; +import { devCheck, lowercaseEventAttribute, unscopedHoleAllocatedIds } from "./diagnostics.js"; export { DOMWithState, ChildProperties, @@ -713,7 +713,10 @@ export function claimElement(node) { export function setAttribute(node: Element, name: string, value: string): void; export function setAttribute(node, name, value) { - if ("_SOLID_DEV_") tagElement(node); + if ("_SOLID_DEV_") { + tagElement(node); + if (typeof value === "function" && name.startsWith("on")) lowercaseEventAttribute(name, node); + } if (isHydrating(node)) return; const selectMultiple = name === "multiple" && node.localName === "select"; if (value == null || value === false) node.removeAttribute(name); @@ -1901,13 +1904,13 @@ function flushHeadRegistry() { } } -// Shared filtered create: skips children/ref/on* and invalid names, drops -// null/false values, sets the text body. Used by the replaceable render and -// the resource mount. +// Shared filtered create: skips children/ref/onXxx handlers and invalid +// names, drops null/false values, sets the text body. Used by the +// replaceable render and the resource mount. function createHeadElement(tag, props) { const el = document.createElement(tag); for (const name in props) { - if (name === "children" || name === "ref" || name.slice(0, 2) === "on") continue; + if (name === "children" || name === "ref" || /^on[A-Z]/.test(name)) continue; if (!HEAD_ATTR_NAME.test(name)) { if ("_SOLID_DEV_") console.warn(`useHead: ignoring invalid attribute name "${name}"`); continue; @@ -1933,7 +1936,7 @@ function renderHeadElement(t, identity, existing) { function headElementMatches(el, t) { if (el.tagName.toLowerCase() !== t.tag) return false; for (const name in t.props) { - if (name === "children" || name === "ref" || name.slice(0, 2) === "on") continue; + if (name === "children" || name === "ref" || /^on[A-Z]/.test(name)) continue; if (!HEAD_ATTR_NAME.test(name)) continue; const v = t.props[name]; if (v == null || v === false) { @@ -2577,9 +2580,13 @@ function assignProp(node, prop, value, prev, skipRef, nodeName) { return value; } + let c; const hasNamespace = prop.indexOf(":") > -1; - if (!hasNamespace && prop.slice(0, 2) === "on") { + // Only `on` + an uppercase letter is an event; a lowercase `onclick` is an + // attribute. No regex on this per-prop path: a key not starting with `on` + // pays one `startsWith`. + if (!hasNamespace && prop.startsWith("on") && (c = prop.charCodeAt(2)) > 64 && c < 91) { const name = prop.slice(2).toLowerCase(); const delegate = DelegatedEvents.has(name); if (!delegate && prev) { diff --git a/packages/web/src/constants.ts b/packages/web/src/constants.ts index 0dfdd30e2..50165b681 100644 --- a/packages/web/src/constants.ts +++ b/packages/web/src/constants.ts @@ -232,6 +232,14 @@ function isHttpNavigationTarget(target: string): boolean { } } +// Only `on` + an uppercase letter (`onClick`) is an event handler. Lowercase +// `on*` names (`onclick`) are plain attributes. No regex: this runs on every +// key of a spread, and a key not starting with `on` pays one `startsWith`. +function isEventName(name: string): boolean { + let c; + return name.startsWith("on") && (c = name.charCodeAt(2)) > 64 && c < 91; +} + export { DOMWithState, ChildProperties, @@ -245,5 +253,6 @@ export { $$SLOT, $$HOST, COMPOSED_BODY_FRAMING, - isHttpNavigationTarget + isHttpNavigationTarget, + isEventName }; diff --git a/packages/web/src/diagnostics.ts b/packages/web/src/diagnostics.ts index 7230a7e7b..da7be7e41 100644 --- a/packages/web/src/diagnostics.ts +++ b/packages/web/src/diagnostics.ts @@ -114,6 +114,32 @@ export function unscopedHoleAllocatedIds( ); } +const lowercaseEventAttributesReported = /*#__PURE__*/ new Set(); + +/** + * Dev CHECK: a function reached an attribute whose name starts with `on` but + * is not an event handler — a lowercase `onclick`, or a 1.x `on:click`. Only + * `on` + an uppercase letter (`onClick`) binds an event in 2.0; anything else + * is a plain attribute, so the function is stringified into the attribute's + * text. Reported once per attribute name so a row template reports once. + */ +export function lowercaseEventAttribute(name: string, node: Element): void { + if (lowercaseEventAttributesReported.has(name)) return; + lowercaseEventAttributesReported.add(name); + const event = name.slice(name.charAt(2) === ":" ? 3 : 2); + const handler = "on" + event.charAt(0).toUpperCase() + event.slice(1); + devCheck({ + code: "LOWERCASE_EVENT_ATTRIBUTE", + kind: "render", + severity: "warn", + message: + `[LOWERCASE_EVENT_ATTRIBUTE] \`${name}\` received a function, but \`${name}\` is an attribute ` + + `in Solid 2.0, not an event handler: the function was set as the attribute's text. ` + + `Use \`${handler}\` for event handlers.`, + data: { name, handler, tag: node.localName } + }); +} + /** `Name: message` for an Error, `String(value)` otherwise. */ export function errorText(error: unknown): string { if (error instanceof Error) return error.message ? `${error.name}: ${error.message}` : error.name; diff --git a/packages/web/src/server.ts b/packages/web/src/server.ts index c2bcaf2ba..cc619ea5d 100644 --- a/packages/web/src/server.ts +++ b/packages/web/src/server.ts @@ -1,5 +1,10 @@ // @ts-nocheck -import { COMPOSED_BODY_FRAMING, ChildProperties, isHttpNavigationTarget } from "./constants.js"; +import { + COMPOSED_BODY_FRAMING, + ChildProperties, + isEventName, + isHttpNavigationTarget +} from "./constants.js"; import { createRoot as root, getOwner, @@ -1371,7 +1376,7 @@ function flushHeadFragment(registry, boundary, nonce) { const t = winner.tags[i]; const attrs = {}; for (const name in t.props) { - if (name === "children" || name === "ref" || name.slice(0, 2) === "on") continue; + if (name === "children" || name === "ref" || isEventName(name)) continue; if (!HEAD_ATTR_NAME.test(name)) { if ("_SOLID_DEV_") headTagInvalid( @@ -1486,7 +1491,7 @@ function nonceAttr(nonce, destination) { function renderHeadAttrHtml(props) { let attrs = ""; for (const name in props) { - if (name === "children" || name === "ref" || name.slice(0, 2) === "on") continue; + if (name === "children" || name === "ref" || isEventName(name)) continue; if (!HEAD_ATTR_NAME.test(name)) { if ("_SOLID_DEV_") headTagInvalid( @@ -1509,7 +1514,7 @@ function renderHeadAttrHtml(props) { function headAttrRecord(props, skipRelHref) { let attrs = null; for (const name in props) { - if (name === "children" || name === "ref" || name.slice(0, 2) === "on") continue; + if (name === "children" || name === "ref" || isEventName(name)) continue; if (skipRelHref && (name === "rel" || name === "href")) continue; if (!HEAD_ATTR_NAME.test(name)) continue; const v = props[name]; @@ -4475,7 +4480,7 @@ export function ssrElement(tag, props, children, needsId, skip, attrs, claims) { // `sourceHas`), then attaches nothing for `undefined` — so a named // handler before this source binds nothing either. if (value == undefined) { - if (slots && claims !== undefined && (prop === "ref" || prop.startsWith("on"))) + if (slots && claims !== undefined && (prop === "ref" || isEventName(prop))) behaviors = spreadBehaviorPosition( behaviors, prop, @@ -4498,7 +4503,7 @@ export function ssrElement(tag, props, children, needsId, skip, attrs, claims) { // that knows them (`spreadObjectAttribute`; the compiled positions // share it); strings and booleans — the walk's common case — never // do, and outside a server component the ladder is the pre-slot one. - if (prop === "ref" || prop.startsWith("on")) { + if (prop === "ref" || isEventName(prop)) { if (slots) behaviors = spreadBehaviorPosition( behaviors, @@ -4544,7 +4549,7 @@ export function ssrElement(tag, props, children, needsId, skip, attrs, claims) { // source it replaces had its getters read: the expressions run at the same // point in the hydration-id sequence. Evaluating it in argument position // would move them ahead of the element's own key. - // `claims` is the compiled claim map of the element's named `ref`/`on*` + // `claims` is the compiled claim map of the element's named `ref`/`onXxx` // attributes (the spread element's counterpart of the template path's // guarded `ssrClaim` hole), keyed by the source index each attribute sits // before, a thunk read only inside a server component's render — the same @@ -4585,7 +4590,7 @@ export function ssrElementAttribute(key, value) { // walk, in the same order: nullish is "not set" (`class`/`style` included, // #3382), `style`/`class` take their serializers, a boolean is present or // absent, `""` is a bare attribute, anything else is attribute-escaped. - // `key` is a compile-time attribute name (never `ref`, `on*` or `prop:*`, + // `key` is a compile-time attribute name (never `ref`, `onXxx` or `prop:*`, // which the compiler drops) and is trusted like `ssrAttribute`'s. // // Under the `serverComponents` compiler option a dynamic `class`/`style` @@ -4613,12 +4618,17 @@ export function ssrAttribute(key, value) { // passes a binding-slot value through untouched, so the position it names // is bound here (principles §9.2.3). Both are trusted here so this hot // path stays a pure string concatenation. + if (typeof value === "string") return ` ${key}="${value}"`; if (value == null || value === false) return ""; if (typeof value === "object") { return isSlotValue(value) ? slotAttribute(key, value) : ` ${key}="${escape(String(value), true)}"`; } + // A function value (`onclick={() => …}`) reaches here with the compiler's + // escape wrapped inside it, not applied. Its source is not attribute-safe: + // stringify and escape it, as the client's setAttribute stringifies it. + if (typeof value === "function") return ` ${key}="${escape(String(value), true)}"`; return value === true ? ` ${key}` : ` ${key}="${value}"`; } export function ssrHydrationKey(): string; @@ -4665,14 +4675,14 @@ export function ssrHydrationKey() { // passes the stand-in through), `ssrElementAttribute` (a compiled // `class`/`style` under the `serverComponents` option, or a trailing // attribute of a spread element), `ssrElement`'s walk (a runtime spread), -// `ssrClaim` (the compiled per-element hole for ref/on* positions). The +// `ssrClaim` (the compiled per-element hole for ref/onXxx positions). The // grammar: the occurrence alphabet (frame-sink.ts) excludes `:`, `,` and // `=`; keys and names percent-encode onto an alphabet that excludes them // too, so every split is exact and the client decodes names back. // The compiled guard's arming values (`sharedConfig.context.claims`): the // frame renderers set one at server-component entry so renders with no -// server components never evaluate the ref/on* hole's expressions. On the +// server components never evaluate the ref/onXxx hole's expressions. On the // document face only owner chains inside the component barrier warn about // server-local handlers (client fill content re-enters the zone owner // captured OUTSIDE the barrier — its handlers are hydration's). @@ -4912,7 +4922,7 @@ function eventPosition(prop) { } /** - * A `ref`/`on*` key met by `ssrElement`'s walk under `slots` — in a + * A `ref`/`onXxx` key met by `ssrElement`'s walk under `slots` — in a * source, whatever its shape: a stand-in, a list of them (refs), a handler * tuple, a server-local function, nothing — collected by position into * `behaviors` exactly as `ssrClaim` reads the compiled claim map, so the @@ -4946,7 +4956,7 @@ function spreadBehaviorPosition(behaviors, prop, value, mode, index, settle) { /** * The behavior markers of a spread element: the sources' (`behaviors`) - * settled against its compiled claim map (`claims` — the named `ref`/`on*` + * settled against its compiled claim map (`claims` — the named `ref`/`onXxx` * attributes, keyed by the index of the source each sits before; a thunk * `ssrElement` calls only under `slots`, so plain SSR never evaluates * them). The marker promises what the client binds, and the client's @@ -5090,7 +5100,7 @@ function slotSpreadSource(tag, source) { } /** - * The compiled per-element hole for ref/on* positions on a server + * The compiled per-element hole for ref/onXxx positions on a server * intrinsic (behind the `serverComponents` compiler option): * `ctx.claims ? ssrClaim({ click: expr, ref: expr2 }) : ""`. The compiler * drops handler and ref expressions from plain SSR output, so this is diff --git a/packages/web/test/lowercase-on-attribute.spec.tsx b/packages/web/test/lowercase-on-attribute.spec.tsx new file mode 100644 index 000000000..152963a75 --- /dev/null +++ b/packages/web/test/lowercase-on-attribute.spec.tsx @@ -0,0 +1,183 @@ +/** + * @jsxImportSource @solidjs/web + * @vitest-environment jsdom + */ +import { afterEach, describe, expect, test, vi } from "vitest"; +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; +import { createSignal, flush } from "solid-js"; +import { assign, render, spread } from "@solidjs/web"; + +// Only `on` + an uppercase letter (`onClick`) is an event handler in 2.0. +// Lowercase `on*` names are plain attributes: compiled, spread and assigned. + +const warnings = (spy: { mock: { calls: unknown[][] } }) => + spy.mock.calls + .map(args => String(args[0])) + .filter(m => m.includes("[LOWERCASE_EVENT_ATTRIBUTE]")); + +describe("lowercase on* names are attributes", () => { + let container: HTMLDivElement; + let dispose: (() => void) | undefined; + + afterEach(() => { + dispose?.(); + dispose = undefined; + container?.remove(); + vi.restoreAllMocks(); + }); + + function mount(fn: () => any) { + container = document.createElement("div"); + document.body.appendChild(container); + dispose = render(fn, container); + return container.firstElementChild as HTMLElement; + } + + test("a compiled lowercase on* expression sets the attribute and binds no event", () => { + const code = "console.log('hi')"; + const button = mount(() => ( + + )); + expect(button.getAttribute("onclick")).toBe(code); + expect((button as any)._$$click).toBeUndefined(); + }); + + test("a dynamic lowercase on* expression is a reactive attribute", () => { + const [code, setCode] = createSignal("first()"); + const button = mount(() => ( + + )); + expect(noKeys(html)).toBe(''); + }); + + test("a camelCase handler still renders nothing beside it", () => { + const html = renderToString(() => ( + '); + }); + + test("a function value is stringified and escaped like the client's setAttribute", () => { + const html = renderToString(() => ( +