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

Hook doesn't allow element to be null #16

Closed
bvanderdrift opened this issue Nov 1, 2019 · 1 comment · Fixed by #23
Closed

Hook doesn't allow element to be null #16

bvanderdrift opened this issue Nov 1, 2019 · 1 comment · Fixed by #23

Comments

@bvanderdrift
Copy link
Contributor

bvanderdrift commented Nov 1, 2019

I'd like to request that element should be allowed to be null, which makes sure initially there is a not a global event listener, there is no TS problems and the hooks are top-level.

Currently the element parameter typings are defined as

element?: HTMLElement

This causes problems in combination with useRef.

The following example:

const someRef = useRef();
useEventListener("click", onClickHandler, someRef.current);

works but will be listening to the click event on the global parameter initially (since current will be undefined in the first call). For example in the browser between useEffect and useLayoutEffect there will be time that the onClickHandler will be triggered when user clicks anywhere on the page.

A solution would be the have initial value of someRef.current be null but this throws a typescript error.

const someRef = useRef(null);
useEventListener("click", onClickHandler, someRef.current); //Type 'null' is not assignable to type 'HTMLElement | undefined'

One could circumvent this by putting the useEventListener in an if statement:

const someRef = useRef(null);
if(someRef.current !== null){
    useEventListener("click", onClickHandler, someRef.current);
}

But this creates the problem that useEventListener is not a top-level hook anymore which is required.

Therefore I'd suggest element is allowed to be null.

See #17 for the PR with my suggested solution.

@alex-malyita
Copy link

Change useEffect to useLayoutEffect

 useLayoutEffect(() => {
   savedHandler.current = handler;
 }, [handler]);

srmagura added a commit to srmagura/use-event-listener that referenced this issue Mar 7, 2020
Fixes donavon#16. This allows `useEventListener` to be used with
`useRef<HTMLElement | null>(null)`.
donavon pushed a commit that referenced this issue Jul 23, 2020
* add compilation test for typescript types

* ts: allow element argument to be null

Fixes #16. This allows `useEventListener` to be used with
`useRef<HTMLElement | null>(null)`.

* update tests to use @testing-library/react-hooks

* Add argument: options = { capture, passive }

This changes the hook's signature to

useEventListener(eventName, handler, element, options)

`options` is an optional argument. `capture` and `passive` are optional
keys in the `options` object. If provided, they are passed directly to
`addEventListener`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants