Callbacks called twice

[thosta-Witt-Gruppe]

• downshift version: Tested with 6.1.12, 7.0.1 and d62ba91
• node version: 16.18.1
• pnpm version: 7.15.0
• react version: 18.2.0

What you did:

We are using the useSelect hook to display a custom dropdown on larger viewpoints and a modal on smaller viewports.
We recently upgraded to React 18.

What happened:

Under certain conditions, the onSelectedItemChange callback is called twice when selecting an item.

Reproduction repository:

Unfortunately, I am unable to provide a reproduction repo at this point.

Problem description:

The problem started to appear when we updated to React 18.
We did some debugging and found that the dispatch function of downshift's reducer is only called once, however, the actual reducer function is called twice under certain circumstances.

According to the React Docs and this issue, reducers should be written in such a way, that them being called twice has no observable effects for the rest of the application.

However, in downshift's case, the state is updated twice with two distinct objects, each containing the exact same information.
This leads to the useEffect being called twice and thus triggering the callback twice.

Suggested solution:

I don't think that we should try to fix the reducer being called twice, due to the lack of a reproduction repo and the fact that React tells developers to expect such behaviour.

Instead, I'd suggest to modify the reducer in such a way, that it being executed twice won't have any observable side effects.
A possible solution might be to replace the following identity check with a deepEqual:

downshift/src/hooks/utils.js

Line 182 in 7540e3d

if (action && prevStateRef.current && prevStateRef.current !== state) {

I.e.:

    if (action && prevStateRef.current && !deepEqual(prevStateRef.current, state)) {

with deepEqual being something like this: https://stackoverflow.com/a/32922084

However, I am not sure if this approach has any unintended consequences.

[silviuaavram]

Hi! Thanks for the feedback! It would have been useful to have a repro scenario, so I can figure out what's happening.

[Fruitseller]

Hello, we have an issue which is a combination of this one and #1384. We were unable to create a reproduction for that, but have a repo which shows that onSelectedItemChange is called twice. Hope the repo helps to resolve this issue.

[trainoasis]

We had the same issue on 7.0.5 & it remains on 7.2.0.

[nilscox]

I had a similar behavior when rendering checkboxes in the menu items, because clicking the label triggers the click event twice on the menu item element. Here is a reproduction of the problem, replacing the <label /> with anything else solves the issue.

While there is no issue with downshift, I thought it might help some people that may come across this issue.

[silviuaavram]

https://codesandbox.io/s/quiet-bash-nyfsqq?file=/src/hooks/useSelect/basic-usage.js

I copied the example from @Fruitseller and I don't get the callback called twice.

Can we get more info so we can dig further? Thanks!

[klnwsk]

@silviuaavram I'm also suffering from this issue. @Fruitseller example reproduces the error callback being called twice but only when you use devtools with mobile option because then 2 events happen on each click. One mouse click event and touch event maybe they collide or something, on real mobile device it's the same, callback called twice.

[okonet]

@silviuaavram it's reproducible when useMultipleSelection is used: https://codesandbox.io/s/musing-lamport-spp6zn?file=/src/hooks/useMultipleSelection/select.js

[silviuaavram]

Everyone, try downshift@8.2.4-alpha.2. If this solves the issue and does not break anything, we will release it as normal 8.2.4. Thank you!

https://www.npmjs.com/package/downshift/v/8.2.4-alpha.2

[FdelMazo]

@silviuaavram, beware that the package was released with a typo! it's downshift@8.2.4-alhpa.0.

(couldn't get a chance to test it yet)

[silviuaavram]

Oups :) you can still test it though and let me know if it works. Thanks!

…

On Sun, 12 Nov 2023 at 22:00, Federico del Mazo ***@***.***> wrote: @silviuaavram <https://github.com/silviuaavram>, beware that the package was released with a typo! it's ***@***.*** (couldn't get a chance to test it yet) — Reply to this email directly, view it on GitHub <#1447 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACWAZAA2WR4Y7UX7ZF27O6LYEFBIFAVCNFSM6AAAAAASLD7O5WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBXGI2TGMBTGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[silviuaavram]

Actually just go for 8.2.4-alpha.2

[markossankey]

Actually just go for 8.2.4-alpha.2

I tried this version, and still seeing the onSelectedItemChange being called twice 😬

edit to add more context - only occurring in mobile views

[silviuaavram]

8.2.4 includes the state deep compare. we also had an issue until right now on mobile with controlled selectedItem via formik which triggered an infinite loop on item select, and it got fixed by the deep state compare + formik update.

If there are still issues with these callback changes, I don't think they are downshift related.

[danichim]

This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops.

[silviuaavram]

Hey everyone! I think I got it.

#1571

I also released the 8.3.2-alpha.0 version if you'd like to test it out. Please also check the details in the PR, see if they make sense, and let me know what you think!

[FdelMazo]

@silviuaavram can confirm this fixed my issue! Thanks!!