fix: avoid registering non-function event-handlers on custom-elements

[JoviDeCroock]

Fixes #4085

Prevent us from registering functions on custom-elements when the property starts with on

[github-actions[bot]]

📊 Tachometer Benchmark Results

Summary

A summary of the benchmark results will show here once they finish.

Results

The full results of your benchmarks will show here once they finish.

_{tachometer-reporter-action v2 for Benchmarks}

[github-actions[bot]]

Size Change: +46 B (0%)

Total Size: 57 kB

Filename	Size	Change
dist/preact.js	4.37 kB	+8 B (0%)
dist/preact.min.js	4.41 kB	+8 B (0%)
dist/preact.min.module.js	4.4 kB	+8 B (0%)
dist/preact.min.umd.js	4.43 kB	+5 B (0%)
dist/preact.module.js	4.4 kB	+10 B (0%)
dist/preact.umd.js	4.44 kB	+7 B (0%)

ℹ️ View Unchanged

Filename	Size	Change
compat/dist/compat.js	3.95 kB	0 B
compat/dist/compat.module.js	3.87 kB	0 B
compat/dist/compat.umd.js	4.01 kB	0 B
debug/dist/debug.js	3.53 kB	0 B
debug/dist/debug.module.js	3.52 kB	0 B
debug/dist/debug.umd.js	3.61 kB	0 B
devtools/dist/devtools.js	232 B	0 B
devtools/dist/devtools.module.js	240 B	0 B
devtools/dist/devtools.umd.js	315 B	0 B
hooks/dist/hooks.js	1.53 kB	0 B
hooks/dist/hooks.module.js	1.56 kB	0 B
hooks/dist/hooks.umd.js	1.62 kB	0 B
jsx-runtime/dist/jsxRuntime.js	360 B	0 B
jsx-runtime/dist/jsxRuntime.module.js	326 B	0 B
jsx-runtime/dist/jsxRuntime.umd.js	441 B	0 B
test-utils/dist/testUtils.js	453 B	0 B
test-utils/dist/testUtils.module.js	454 B	0 B
test-utils/dist/testUtils.umd.js	536 B	0 B

_{compressed-size-action}

[JoviDeCroock]

The failure here is interesting, basically we have a fallthrough case now that we shield for the value being a function in either the old or new case. When we are hydrating and have the test where we perform onClick={null} then we have no oldValue (undefined) and no newValue (null) so we fall to the removeAttribute case

[developit]

I worry that this will prevent us from supporting EventListener Objects in the future, in addition to the issue you noted with set/unset.

An idea I had while looking at this: what if we hoisted the in check up above on* handler detection? This would cause onclick to be set as a property, but folks aren't really using lowercase DOM handler names anyway.

if (name === 'style') {
  // set/diff style
  return;
}

if (
  !isSvg &&
  name !== 'width' &&
  name !== 'height' &&
  name !== 'href' &&
  name !== 'list' &&
  name !== 'form' &&
  name !== 'tabIndex' &&
  name !== 'download' &&
  name !== 'rowSpan' &&
  name !== 'colSpan' &&
  name in dom
) {
  try {
    dom[name] = value == null ? '' : value;
    return;
  } catch (e) {
    // property assignment failed, fall back to attribute
  }
}

if (name[0] === 'o' && name[1] === 'n' && name.indexOf('-') === -1) {
  // if we got here, we know `onxyz` is not a property defined on `dom` and there was no dash
  // set/diff event listener
  return;
}

if (typeof value === 'function') return;

// fall through to setting as attribute
if (value != null && (value !== false || name[4] === '-')) {
  dom.setAttribute(name, value);
} else {
  dom.removeAttribute(name);
}

[JoviDeCroock]

@developit the issue with that approach is that you'll fall into the on case for only-overflow for instance. Checking whether we are dealing with a custom-element on that branch seems wrong as we can't support event-handlers on custom-elements like that. We could do a check like the event not containing a dash but if they then leverage onlyoverflow it would also fall in this bucket 😅

Not too sure how we can support all three here

<x-custom onChange={myFunc} only-overflow onlyoverflow />

Where the first has to fall in our event-handler bucket and the latter two in our setProperty