Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: EventTarget #246

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

Symbitic
Copy link

This RFC proposes making React components more web-like by defining a dispatchEvent and addEventListener prop in every React component, and a useEventTarget hook to use them.

View formatted RFC

@EECOLOR
Copy link

EECOLOR commented Apr 11, 2023

I do not understand how it is an improvement over the onEventName version. Your example would be simplified to the following:

function ParentComponent() {
  const [text, setText] = useState("Count: 0");

  return (
    <ChildComponent onCountChange={count => setText(`Count: ${count}`)}>
      <h1>{text}</h1>
    </ChildComponent>
  );
}

function ChildComponent({ children, onCountChange }) {
  const [count, setCount] = useState(0);

  return (
    <>
      {children}
      <button onClick={() => onCountChange(count-1)}>-</button>
      <button onClick={() => onCountChange(count+1)}>+</button>
    </>
  );
}

@Symbitic
Copy link
Author

I do not understand how it is an improvement over the onEventName version.

This is more web-like. DOM Elements have addEventListener and dispatchEvent. React components should have them too.

In larger components, it also means not having to pass multiple onEventName props.

@RexSkz
Copy link

RexSkz commented Apr 12, 2023

I have three questions about eventTarget:

  1. It looks like a prop and may be confusing for newcomers.
  2. It may change the fiber implementation.
  3. It may not work well when ChildComponent is not rendered (due to some conditions).

Event bubbling works like context, context doesn't have these issues, and we can use context to achieve the goal. Please see this demo in CodeSandbox.

@Symbitic Symbitic closed this Apr 12, 2023
@Symbitic Symbitic reopened this Apr 12, 2023
@Symbitic
Copy link
Author

The goal is to be more web like. DOM Elements have EventTarget built-in, despite it being something that could be implemented in 1-2 KB of userland code. Likewise, React Context could replicate this small example, but it doesn't solve the issue of making React components be more web like.

You'd also end up with a pretty big context in a large application.

@EECOLOR
Copy link

EECOLOR commented Apr 14, 2023

This is more web-like. DOM Elements have addEventListener and dispatchEvent. React components should have them too.

The goal is to be more web like.

I am not yet convinced why being 'more web-like' should be a goal. The web has some features that would not have been there if designed again from the ground up. React has a certain structure and way of working that proved successful in more efficiently creating maintainable applications. Part of the reason it works better than plain HTML / JavaScript is that it requires you to work differently than the traditional built-in way.

Again, stating it should be 'more web-like' is not an argument in itself. Please provide compelling use-cases where your approach clearly makes things more succinct or readable.

@Symbitic
Copy link
Author

I disagree about web-like a reason by itself. React native recently had an RFC (react-native-community/discussions-and-proposals#607) to add more compatibility with the web. Deno came about specifically because Node wasn't following web standards, which encouraged Node developers to begin implementing more web standards.

That said, the primary use case for this is the same use case for EventTarget: decoupling state and events. The observer pattern is near ubiquitous in GUI programming toolkits for a reason. With RSC, React is transitioning from the "view" in MVC to an application architecture. It needs a better way of responding to events than passing count/setCount to every child component.

@lubieowoce
Copy link

lubieowoce commented Apr 30, 2023

React native recently had an RFC (react-native-community/discussions-and-proposals#607) to add more compatibility with the web. Deno came about specifically because Node wasn't following web standards, which encouraged Node developers to begin implementing more web standards.

To my eye, the argument for those is mostly interoperability. Like, for me, the main argument for aligning with web standards in those cases is to be able to use the same set of API's across server/browser/RN. I don't really see that gain here, and TBH it looks quite cumbersome compared to plain callbacks.

If you're arguing that React conventions should look more like the native browser APIs, i think you need to motivate that more. Perhaps if you could provide some examples of how this shift could help with e.g. interop with browser API's or something, that'd make the case stronger?

(I'm also saying this because i find the addEventListener family of APIs pretty clunky, and i consider react-style onFoo callbacks a DX improvement. i think many others do as well. so i think you need a strong case for why this is desirable.)

@GeorgeTaveras1231
Copy link

I was drawn to this RFC because I've spent a lot of time thinking about this while working in a component library.

I can understand @Symbitic's point about being more web-like. I think that is something novel to strive for; though I think more can be done to clarify the gains of doing that. My position is, I want to build APIs that are more web-like to reduce the learning time for developers -- because ideally, the APIs resemble some concept in the web development that is already familiar to them. The other case is the case of interoperability. Though there can always be libraries that serve as adapters connecting APIs, libraries can be developed targeting known web APIs, and re-purposed ("easily"?) to work in react.

However, I also agree with @lubieowoce's point about the EventTarget API feeling clunky. The proposal in the RFC works against a much simpler way of handling custom events (the on$NAME callback convention) and works against the simplicity that React offers.

With that said, I'd like to call attention to an RFC I've opened that has some overlap with this one. @Symbitic (and others in this thread), I would be interested in hearing your thoughts on it

RFC: Custom Events #254

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants