facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
229.9k stars 47.07k forks source link

[React 19] custom element property vs. event handler for props beginning with "on" #29659

Closed cjpillsbury closed 1 week ago

cjpillsbury commented 6 months ago

Summary

Per discussion in issue #11347 and in PR #22184, the current implementation of custom element support in React 19 does the following:

  1. checks if the react prop name begins with on
  2. checks if the react prop value is of type function

If both conditions hold, it will addEventListener for an event based off of the remaining name, also using naming conventions to check if the suffix is Capture to determine whether or not to use capture phase for the event.

Example:

<my-custom-element
  // This will listen for events of type `"customevent"`
  oncustomevent={(evt) => console.log(evt.type)}
  // This will listen for events of type `"customevent"` and will `useCapture`(See: https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#usecapture)
  oncustomeventCapture={(evt) => console.log(event.eventPhase, event.type)}
/>

In the aforementioned discussions, there was some debate on how to approach events, namely:

I originally proposed that we do the same thing as preact here (forward everything starting with "on" to addEventListener), but if I remember correctly Sebastian felt strongly that we shouldn't reserve everything starting with "on" for event listeners. Jason Miller also felt strongly about this the other way, so maybe it's something that I can revisit/discuss more after merging this PR - maybe more users will chime in if they get to test it out.

Referencing this, @jfbrennan called out, among other things, the relevant bits of the HTML Standard (See: https://github.com/facebook/react/issues/11347#issuecomment-1767301646)

I believe there is a simple path forward to strike a balance here while still having support that is "idiomatically React"

Proposal

Prefer the "in heuristic" even for properties that meet the two conditions mentioned above over adding an event listener.

Motivation

Relevant code

https://github.com/facebook/react/blob/main/packages/react-dom-bindings/src/client/DOMPropertyOperations.js#L194-L222

cc @jakearchibald @rickhanlonii for visibility

cjpillsbury commented 6 months ago

Oh also worth mentioning, I'm happy to issue a PR for these changes if folks are on board.

trusktr commented 3 months ago

The most ideal solution would be if JSX were updated to have syntax that lets users be 100% in control. It would be similar to Lit's html syntax (in effect, not necessarily the with matching the same syntax), which gives users the ability to pick whether a prop is setting an attribute, JS property, boolean attribute, or event listener.

In Lit, it is like this:

return html`
  <some-el
    foo="this sets the foo attribute (no guessing)"
    .bar="this sets the bar JS property (no guessing)"
    ?baz=${someBool/*this adds or removes the baz attribute based on a boolean (no guessing)*/}
    @lorem=${() => {/*this adds a listener for the lorem event (no guessing)*/}}
  ></some-el>
`

In other JSX frameworks, the following syntax features have been established:

return <some-element
  foo={/*may use heuristics to set a foo attribute or foo JS property*/}
  attr:bar={/*set the bar attribute (no guessing)*/}
  prop:baz={/*set the baz JS property (no guessing)*/}
  bool:boom={/*adds or remove the boom attribute based on a boolean (no guessing)*/}
  on:some-event={/*add a listener for some-event (no guessing)*/}
/>

For example, see Pota by @titoBouzout which provides that syntax for its alternative JSX compiler for Solid.js. Pota also provides an html template tag with the same syntax. Additionally, Pota's JSX and html templates both support Lit's .foo, ?foo, and @foo syntaxes.

Solid's JSX has attr:, prop:, and on:, but it is currently missing bool:.

The above approach would make it possible for users to be able to do what they want to do 100% of the time, with no guessing, by being able to select the syntax they need for the particular element.

There is no way that a heuristic-based approach can allow 100% of user cases to be satisfied, but with a fairly simple syntax addition, all users could be satisfied.

Trying to make the heuristics-based approach cover everyone's needs is, most likely, impossible.


Another way to look at it: instead of having rules that would make elements have to conform to React, allow React templating to freely express all that is possible with elements.

jfbrennan commented 3 months ago

Expect web component authors to have a 2nd property whose name does not begin with "on" that mirrors the functionality of the property whose name does begin with "on".

In other words, expect authors to define valid APIs, which is the right approach. Frameworks should not start supporting invalid attribute/property naming. "on" always means event handler. WC authors need to know that and follow that regardless of tech stack. As for syntax, JSX doesn't need to do anything different than it already does.

github-actions[bot] commented 2 weeks ago

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

github-actions[bot] commented 1 week ago

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!