fix: devtools to patch api.setState

[dai-shi]

Close #171.

There are a few technical issues:
1. we need to fix types in vanilla.ts and index.ts too.
2. we can't chain middleware that only patches set.

Due to 2), I'm a bit hesitant to this approach.

[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 c981883:

Sandbox	Source
React	Configuration
React Typescript	Configuration
bold-fast-6hsx3	Issue #171

[dai-shi]

@TkDodo Here's your example with this change. https://codesandbox.io/s/agitated-stonebraker-bg7bi

[dai-shi]

Okay, this f4c6239 should be much better.
https://codesandbox.io/s/wonderful-lovelace-rjego

[TkDodo]

Thanks! From what I can tell, the code looks good. I understand that we cannot change the interface of set, so there won't be the possibility to give those "actions" a name, but this is an understandable limitation 👍

[dai-shi]

@TkDodo Did you try the sandbox?
It would be nice if you could try and find possible bugs.

we cannot change the interface of set, so there won't be the possibility to give those "actions" a name

By set, do you mean the first argument of the creator function, or api.setState?

[TkDodo]

Did you try the sandbox?
It would be nice if you could try and find possible bugs.

Yes, I did try the codesandbox and it works fine for me. I made a test case for testing it in combination with the redux middleware: https://codesandbox.io/s/react-forked-xbjg9

From what I can see, the INCREASE action is logged twice (once as action and once as INCREASE). This is the same as the previous behaviour, so this PR didn't introduce this. However, setState is now logged as well (wasn't logged before), so that's good. Is this worth a follow up issue maybe?

By set, do you mean the first argument of the creator function, or api.setState?

I mean api.setState. set in the creator function is patched / typed so that we have a third argument "name", but that's not the case for api.setState

[dai-shi]

Thanks!

From what I can see, the INCREASE action is logged twice

Maybe, I can fix it in this PR.

I mean api.setState. set in the creator function is patched / typed so that we have a third argument "name", but that's not the case for api.setState

Yeah, totally correct. Let's leave it as a limitation because types would be too messy, if we were to support this. (I think it's technically possible.)

If you are adventurous, it might be able to do like this:

const useStore = create(devtools((set, get, api) => {
  api.setState = set
  return {
    // ...
  }
}))

[dai-shi]

https://codesandbox.io/s/react-forked-6czzc
Looks good?

Oh, but this will stop logging if you use set with redux middleware.

[TkDodo]

Yeah, totally correct. Let's leave it as a limitation because types would be too messy, if we were to support this. (I think it's technically possible.)

yes, sounds good.

If you are adventurous, it might be able to do like this:

Yes, I am actually doing that already as a workaround until this PR lands:

let setState: SetState

const useState = create<State>(
    devtools((set) => {
        setState = set
        return { ... }
}))

Oh, but this will stop logging if you use set with redux middleware.

hm, how would you even do that ? In the example, we only pass the reducer to redux, and useState.setState is now correctly logged ... 🤔

[dai-shi]

This 99a9f64 is probably better, even thought it's unlikely that people use set with redux middleware. (This means another middleware between devtools and redux.)

[dai-shi]

@TkDodo Would you do the final test? If it's OK, we release it.

[TkDodo]

using the redux example with 99a9f64 now logs increase button twice, first as setState and then as INCREASE

https://codesandbox.io/s/angry-mestorf-qymx1

[dai-shi]

Oops!

[dai-shi]

Reverted. 😄

https://codesandbox.io/s/react-forked-661ih

[TkDodo]

👍

[dai-shi]

Published: https://www.npmjs.com/package/zustand/v/3.0.3