react-transition-group callbacks called with undefined argument

[iamhosseindhv]

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Some callbacks (e.g. onExited) are called with undefined argument.

props.onExited(node, undefined)

Expected Behavior 🤔

Callbacks to be called with the right arguments accordingly

props.onExited(node)
props.onEntered(node, isAppearing)

Steps to Reproduce 🕹

https://codesandbox.io/s/material-demo-ghrv2

Steps:

1. Toggle the "show" switch twice
2. check the console. Callback is called with node and undefined arguments.

Context 🔦

It may not be as important if people are using the callbacks as is, but in notistack I need to call the callbacks with an additional argument (snackbar's unique id). And having an undefined agument creates a gap.

This caused a bug in latest release of notistack. Where I was expecting to receive snackbar's unique key as the second argument, but I was receiving it as the third argument. The fix was released as a temporary solution.

Issue is introduced in #20952. More specifically, any instances in the following form should be fixed: example. Alternatively normalizedTransitionCallback can be modified.

- const handleExited = normalizedTransitionCallback(onExited);
+ const handleExited = normalizedTransitionCallback((node) => {
    onExited(node)
});

The reason being normalizedTransitionCallback always calls the callback with 2 arguments. This is the case for some callbacks which have isApearing as the second argument. But don't apply to others (e.g. onExited) where it's called with one argument (that being node).

Happy to create a PR once the bug is confirmed.

Your Environment 🌎

Tech	Version
Material-UI	v4.9.14
React	v16.12.0
Browser	Chrome
TypeScript	v3.8.3

[eps1lon]

Thanks for the detailed issue report.

If I understand it correctly this is an issue with notistack that is now fixed?

[iamhosseindhv]

Not exactly.

The behaviour doesn't match typings. As an example here's the type of onExited from @types/react-transition-group:

export type ExitHandler = (node: HTMLElement) => void;

While what's actually is happening is this:

props.onExited(node, undefined)

[eps1lon]

export type ExitHandler = (node: HTMLElement) => void;

Which means any additional parameters are undefined

props.onExited(node, undefined)

which they are. I don't understand how this is an issue unless you count arguments which is something that is very brittle until you own the callsite. E.g. props.onExited(...parameters) would set arguments.length to 0.

[iamhosseindhv]

As you mentioned, I think there's a difference between how it currently is:

const onExited = (...args) => {
    console.log(args.length); // 2
    console.log(args[0], args[1]); // node, undefined
};

with how it should be:

const onExited = (...args) => {
    console.log(args.length); // 1
    console.log(args[0], args[1]); // node, undefined
};

Although I can get around it in my case. Feel free to close the issue 👍🏼

[eps1lon]

If you want to spend time on the issue then go ahead. It's open source after all. But we won't spend time on it since it looks like this is a dev only warning about a possible extraneous parameter that can be fixed in userland.

[iamhosseindhv]

Sounds fair 👍🏼I'll open a PR in the upcoming days