feat(utils): cache redux extension connections to prevent duplicates

[PrettyCoffee]

Originated from this discussion:
pmndrs/jotai#1795 (reply in thread)

Notes:

• I assumed that you accidentally deleted the husky prepare script in the package.json and therefore added it again.
• I changed the initial state value from useAtomsDevtools to an "empty" devtools state sicne that felt more natural to me. Let me know if you still prefer the undefined and I will change it back.
• If you would prefer me to split the PR into smaller ones to reduce the size, let me know. I feel bad for this rather big chunk but was not sure what workflows you prefer 😅

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit eef4381:

Sandbox	Source
React	Configuration
React Typescript	Configuration

[dai-shi]

I'm not sure if I understand the problem in the first place.
Yeah, the PR is very big and can't get it at a glance.

Is it only an issue with StrictMode? And, do yo know how react-redux solves it?
This doesn't feel right, because we never received such issues in zustand, which does something similar. (Wait, zustand doesn't use hooks, so it's jotai specific. Hmmm 🤔 )

[dai-shi]

Not sure if we used husky? (I avoid it in jotai.)

[arjunvegda]

We use husky for devtools to enforce conventional commits but we don't need the prepare script. So we can safely omit this. See comment below

[PrettyCoffee]

Hey, may I ask why you don't need the prepare script?
As I see it, the prepare script is required when a developer is pulling a fresh copy of the repo. It will then install the githooks you got in your .husky directory into the .git directory when executing npm i and the dev would not have to execute npx husky install by themselves.
And when you don't have the prepare script, a new contributor (like me) could commit with any message and no validation.

It probably doesn't / rarely matters for you as active maintainers, but I guess it could help new contributors :)

Or, if you don't want to enforce it and it should just be an optional convention, it also makes sense to remove it.

[arjunvegda]

You're absolutely right! We do need the prepare script. I was under the impression that husky auto-installs itself. It turns out they stopped doing that long ago.

[arjunvegda]

Hey, @PrettyCoffee thanks a lot for your contribution!

Sorry, I missed the discussion you filed on the Jotai repo. If I understand correctly, this "bug" is caused by React 18's new approach to enforce idempotency in Strict mode. It runs hooks like useEffect and useMemo twice in development mode. See: https://beta.reactjs.org/reference/react/useEffect#my-effect-runs-twice-when-the-component-mounts

So making the useAtomDevtools hook idempotent should address the bug (i.e. each connection must also disconnect).

Please see this fix: #33
Here is the fork of your CodeSandbox with the fix: https://codesandbox.io/p/sandbox/jotai-redux-devtools-forked-2dfvcv

⚠️ Note

I think the bigger issue you caught is that the atom values are not being updated

Did you get a chance to investigate why that could be happening?

[PrettyCoffee]

Is it only an issue with StrictMode? And, do yo know how react-redux solves it? This doesn't feel right, because we never received such issues in zustand, which does something similar. (Wait, zustand doesn't use hooks, so it's jotai specific. Hmmm 🤔 )

Yess, thats actually the problem, due to the render cycles it could be executed multiple times which should not be happening.
I guess zustand, as well as react-redux, are only using it as a middleware when initializing the store, therefore it will only be executed once in the apps live-time.

My gut feeling says it could be more beneficial to do the exact same with atoms: Having it as a middleware or something like that instead of a hook. But I am not really deep into jotai's mechanics, so there might be reasons why you wouldn't want to do that.

[PrettyCoffee]

Hey, @PrettyCoffee thanks a lot for your contribution!

Thank you for jotai and having devtools, really feels like a blessing! :)

So making the useAtomDevtools hook idempotent should address the bug (i.e. each connection must also disconnect).

There lies the problem: You cannot do that with redux devtools (at least I didn't find a proper way for that)

Please see this fix: #33 Here is the fork of your CodeSandbox with the fix: codesandbox.io/p/sandbox/jotai-redux-devtools-forked-2dfvcv

I will have a closer look on that later. But I think that this will actually introduce bugs 😕

The __REDUX_DEVTOOLS_EXTENSION__.disconnect() function actually removes all existing connections (devtools in the current code). This works for useAtomsDevtools if you are only using this one and not more of the useAtomDevtools or useAtomsDevtools hooks!
And it does not work for the useAtomDevtools hook since that would disconnect all other useAtomDevtools instances as well, while you probably only want to disconnect one.

According documentation:
https://github.com/reduxjs/redux-devtools/blob/main/extension/docs/API/Methods.md#disconnect

I did not find a function to actually disconnect one connection from the extension. It is all or none.

I think the bigger issue you caught is that the atom values are not being updated
Did you get a chance to investigate why that could be happening?

Unfortunatly I didn't find the problem there. Could be that it lies within the other cleanup issues I addressed, but I am not entirely sure. If I got the time I can have a closer look at this again. 👍

[arjunvegda]

I will have a closer look on that later. But I think that this will actually introduce bugs 😕

I'm unfamiliar with the internals of Redux DevTools Extensions, so could you help us create a small repro of the issues you're referring to?

I added useAtomsDevtools("foo-store"); on top of useAtomDevtools(atom) but was unable to reproduce the issue.

Unfortunatly I didn't find the problem there. Could be that it lies within the other cleanup issues I addressed, but I am not entirely sure. If I got the time I can have a closer look at this again. 👍

Found the issue, this is happening because <DebugAtoms/> is located outside of the Jotai Provider. Moving it within the <Provider/> fixes it. See updated CSB - https://codesandbox.io/p/sandbox/jotai-redux-devtools-forked-2dfvcv

[PrettyCoffee]

I'm unfamiliar with the internals of Redux DevTools Extensions, so could you help us create a small repro of the issues you're referring to?

I will add a small documentation to the redux tool files and submit them in a separate PR, then we can discuss that detail there 👍

I added useAtomsDevtools("foo-store"); on top of useAtomDevtools(atom) but was unable to reproduce the issue.

What I meant there is, that when you

1. render the useAtomsDevtools
2. render the useAtomDevtools
3. unrender the useAtomsDevtools
4. extension.disconnect() is executed on cleanup

you will loose all connections that are existing, even tho theoretically some should be there.

Tweaked example

Found the issue, this is happening because <DebugAtoms/> is located outside of the Jotai Provider. Moving it within the <Provider/> fixes it. See updated CSB - https://codesandbox.io/p/sandbox/jotai-redux-devtools-forked-2dfvcv

Aaaah, yes, that makes sense! Thank you for fixing :)

[PrettyCoffee]

Added an issue in redux-devtools to see if they got a solution for the problem:
reduxjs/redux-devtools#1370