reactjs / react-transition-group

An easy way to perform animations when a React component enters or leaves the DOM
https://reactcommunity.org/react-transition-group/
Other
10.18k stars 649 forks source link

Undefined is passed into node arg of Transition callbacks contrary to type definition #908

Closed sydneyjodon-wk closed 2 months ago

sydneyjodon-wk commented 2 months ago

What is the current behavior?

Hello! 👋

I'm not sure if I should make this issue here or in https://github.com/DefinitelyTyped/DefinitelyTyped, but I'm starting here.

We are trying to use strict null safety and are noticing that undefined is sometimes being passed in for the node arg on onExit, addEndListener, and other related exit/enter callbacks, but the type definition says that node will never be nullable:

export type EndHandler<RefElement extends undefined | HTMLElement> = RefHandler<
    RefElement,
    (node: HTMLElement, done: () => void) => void,
    (done: () => void) => void
>;

export type EnterHandler<RefElement extends undefined | HTMLElement> = RefHandler<
    RefElement,
    (node: HTMLElement, isAppearing: boolean) => void,
    (isAppearing: boolean) => void
>;

export type ExitHandler<E extends undefined | HTMLElement> = RefHandler<E, (node: HTMLElement) => void, () => void>;

Source: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/6ae2e4769cf67e9e4527fa881116c3d58b43324d/types/react-transition-group/Transition.d.ts#L12-L24

What is the expected behavior?

I'm wondering if either undefined shouldn't be passed in for node, or if the typing can be updated to represent the behavior maybe something like this (not sure if that's the right way to solve it):

export type EndHandler<RefElement extends undefined | HTMLElement> = RefHandler<
    RefElement,
-    (node: HTMLElement, done: () => void) => void,
+    (node: RefElement, done: () => void) => void,
    (done: () => void) => void
>;

export type EnterHandler<RefElement extends undefined | HTMLElement> = RefHandler<
    RefElement,
-    (node: HTMLElement, isAppearing: boolean) => void,
+    (node: RefElement, isAppearing: boolean) => void,
    (isAppearing: boolean) => void
>;

export type ExitHandler<E extends undefined | HTMLElement> = 
-    RefHandler<E, (node: HTMLElement) => void, () => void>;
+    RefHandler<E, (node: E) => void, () => void>;

Could you provide a CodeSandbox demo reproducing the bug?

https://codesandbox.io/p/sandbox/transition-undefined-node-arg-g24fgt?file=%2Fsrc%2FDemo.tsx

Thank you!

sydneyjodon-wk commented 2 months ago

Nevermind, I think this is actually hitting this explicit type and there's actually no args being passed through