Skip to content

Mirror of facebook react#17651 #1

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

Open
wants to merge 1 commit into
base: pr17651-dst
Choose a base branch
from
Open

Conversation

smagill
Copy link
Owner

@smagill smagill commented Dec 20, 2019

Mirror of facebook react#17651
Note: This API is intentionally meant to be a low-level way of creating events and assigning listeners to them. It's meant to be verbose so larger building blocks can be created on top of them.

This PR is an alternative solution and system to that of my other PR: facebook#17508. Specifically, based off feedback internally, I've tried to tackle some of the problems that were brought up with the createListener approach in facebook#17508:

  • createListener largely depended on events being registered in the commit phase, meaning that there would likely be issues around needing to flush more to ensure we register DOM events. The new approach enforces all events are registered unconditionally via hooks in the render phase, mitigating this issue.
  • createListener allowed for listeners to update and change their properties between renders, which again is problematic for performance and would also require more flushes to ensure we have committed the latest version of each listener.
  • createListener had a complex diffing process to ensure we stored the latest listeners, but this meant that the process had additional overhead and memory usage – which is no longer the case with this PR.
  • createListener required listeners to be put on nodes via the listeners prop. Furthermore, it required using arrays to combine multiple listeners, which some felt was not idealistic and might be confusing during debugging as to which listeners occured at which stages. Also, there was general dislike to introducing another internal prop – as it would mean we'd have to first forbid listeners and wait for React 17 to introduce these APIs, as they might be used in the wild for other reasons (custom elements).
  • createListener didn't provide an idiomatic way to conditionally add/remove root events (they're called delegated events in this new PR). With the new approach, there's a streamlined approach to ensure this is easier to do, and by default, no root events can be added unconditonally, which is a code-smell and a good cause of memory leaks/performance issues.

Taking the above points into consideration, the design of this new event system aims at bringing the same capabilities as described in facebook#17508 whilst also providing some other nice features, that should allow for bigger event sub-systems to be built on top.

ReactDOM.useEvent

This hook allows for the registration to a native DOM event, similar to that of addEventListener on the web. useEvent takes a given event type and registers it to the DOM then returns an object unique to that event that allows listeners to be attached to their targets in an effect or another event handler. The object provides three different methods to setup and handle listeners:

  • setListener(target: document | element, listener: Event => void) set a listener to be active for a given DOM node. The node must be a DOM node managed by React or it can be the document node for delegation purposes.
  • deleteListener(target: document | element) delete the listener for the given DOM node
  • clear() remove all listeners

The hook takes three arguments:

  • type the DOM event to listen to
  • options an optional object allowing for additional properties to be defined on the event listener. The options are:
    • passive provide an optional flag that tells the listener to register a passive DOM event listener or an active DOM event listener
    • capture provide an optional flag that tells thge listener callback to fire in the capture phase or the bubble phase
    • priority provide an optional Scheduler priority that allows React to correct schedule the listener callback to fire at the correct priority.

Note

For propagation, the same rules of stopPropgation and stopImmediatePropgation apply to these event listeners. These methods are actually monkey-patched, as we use the actual native DOM evnets with this system and API, rather than Synthetic Events. currentTarget and eventPhase are also respectfully monkey-patched to co-ordinate and align with the propagation system involved internally within React.

Furthermore, all event listeners are passive by default. If is desired to called event.preventDefault on an event listener, then the event listener should be made active via the passive option.

Examples

An example of a basic clickable button:

import {useRef, useEffect} from 'react';
import {useEvent} from 'react-dom';

function Button({children, onClick}) {
  const buttonRef = useRef(null);
  const clickEvent = useEvent('click');

  useEffect(() => {
    clickEvent.setListener(buttonRef.current, onClick);
  });

  return <button ref={buttonRef}>{children}</button>;
}

If you want to listen to events that are delegated to the document, you can do that:

import {useRef, useEffect} from 'react';
import {useEvent} from 'react-dom';

function Button({children, onClick}) {
  const clickEvent = useEvent('click');

  useEffect(() => {
    // Pass in `document`, which is supported with this API
    // Note: `window` is not supported, as React doesn't
    // listen to events that high up.
    clickEvent.setListener(document, onClick);
  });

  return <button>{children}</button>;
}

If you wanted to extract the verbosity out of this into a custom hook, then it's possible to do so:

import {useRef, useEffect} from 'react';
import {useEvent} from 'react-dom';

function useClick(onClick) {
  const buttonRef = useRef(null);
  const clickEvent = useEvent('click');

  useEffect(() => {
    clickEvent.setListener(buttonRef.current, onClick);
  });

  return buttonRef;
}

function Button({children}) {
  const buttonRef = useClick(() => {
    console.log('Hello world!')
  });
  return <button ref={buttonRef}>{children}</button>;
}

A more complex button that tracks when the button is being pressed with the mouse:

import {useRef, useEffect} from 'react';
import {useEvent} from 'react-dom';

function Button({children, onClick}) {
  const buttonRef = useRef(null);
  const [pressed, setPressed] = useState(false);
  const click = useEvent('click');
  const mouseUp = useEvent('mouseup');
  const mouseDown = useEvent('mousedown');

  useEffect(() => {
    const button = buttonRef.current;

    const handleMouseUp = () => {
      setPressed(false);
    };
    const handleMouseDown = () => {
      setPressed(true); 
    };

    clickEvent.setListener(button, onClick);
    if (pressed) {
      mouseUp.setListener(button, handleMouseDown);
    } else {
      mouseDown.setListener(button, handleMouseDown);
    }

    return () => {
      mouseDown.deleteListener(button);
      mouseUp.deleteListener(button);
    };
  }, [pressed]);

  return (
    <button ref={buttonRef} className={pressed && 'pressed'}>
      {children}
    </button>
  );
}

Work in progress

This PR is still a work-in-progress, and priorities relating to how flushing and controlled components still needs to be worked out. I also intend to add far more tests and ensure that some of the React Flare responders we created in the past, can be modelled on top of this API and redesign.

Update error codes

fix lint

DCE

fix test

Add event kinds to help propagation rules

Add event kinds to help propagation rules #2

Fix kind bug

cleanup

Address feedback

Fix flow

Major refactor and re-design

More revisions

Cleanup

Cleanup 2

Add DCE and unmounting logic for host instances
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants