Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[devtools] doesn't log actions since v2 #77

Closed
pzi opened this issue Nov 4, 2019 · 20 comments · Fixed by #167
Closed

[devtools] doesn't log actions since v2 #77

pzi opened this issue Nov 4, 2019 · 20 comments · Fixed by #167
Labels
bug Something isn't working

Comments

@pzi
Copy link

pzi commented Nov 4, 2019

When updating from 1.0.7 to 2.2.1, I lose the tracking of all actions in the Redux devtools:

2.2.1

image

1.0.7

image

The app works just fine and the store updates correctly when using the app, it just doesn't log out the commits any more.

Using it like:

import create, { StateCreator } from 'zustand'
import { devtools } from 'zustand/middleware'

export const DefaultStoreState: DefaultStateGetters = {
  lastDataUpdate: new Date(),
  loading: false,
  showDrawer: false,
  message: null,
  ...
}

const store: StateCreator<DefaultState> = (set, get) => ({
  ...DefaultStoreState,
  ...
})

export const [useStore, Store] = create<DefaultState>(devtools(store, 'WACHS App Store'))

Any pointers would be appreciated. Let me know if you need more detail and I will update this post.

Cheers & thanks
Patrik

@pzi pzi changed the title devtools doesn't log actions devtools doesn't log actions since v2 Nov 4, 2019
@ghost
Copy link

ghost commented Nov 8, 2019

Hey @pzi,

As you can see on the line https://github.com/react-spring/zustand/blob/master/src/middleware.ts#L33, you need to put name of your function that you want to track in Redux dev tools.

For example:

const [useStore] = create(devtools(set => ({
  count: 1,
  inc: () => set(state => ({ count: state.count + 1 }), 'inc'),
  dec: () => set(state => ({ count: state.count - 1 }), 'dec')
})))

I had the same problem too! Hope this help.

@pzi
Copy link
Author

pzi commented Nov 8, 2019

thanks for the suggestion @VuHG, however, it seems that the types in typescript are not up-to-date as it's complaining about set only accepting 1 argument, not 2.

image

@drcmda
Copy link
Member

drcmda commented Nov 8, 2019

do you know what types it would need? im still struggling with TS. i can update and cut a release to fix this.

@ghost
Copy link

ghost commented Nov 8, 2019

Can you try this this @pzi:

import create, { State, PartialState } from 'zustand'
import { devtools } from 'zustand/middleware'

type NamedSetState<T extends State> = (partial: PartialState<T>, name?: any) => void

interface Count {
  count: number
}

const [useStore] = create<Count>(devtools((set: NamedSetState<Count>) => ({
  count: 1,
  inc: () => set((state: any) => ({ count: state.count + 1 }), 'inc'),
  dec: () => set((state: any) => ({ count: state.count - 1 }), 'dec')
})));

@drcmda, should we export this in middleware.ts

export type NamedSetState<T extends State> = (partial: PartialState<T>, name?: any) => void

@drcmda
Copy link
Member

drcmda commented Nov 8, 2019

i guess, or should it fallback to some default name = "action" or something?

would you make the PR?

@ghost
Copy link

ghost commented Nov 8, 2019

Hey drcmda, i'm struggling with TS too 😅 .
I can't get (partial: PartialState<T>, name: any = "action") fallback.

This is popular library (can affect a lot of devs) so i think it better to wait for some TS expert helping us here.

@pzi
Copy link
Author

pzi commented Nov 13, 2019

Yeah with the type override it works :) Will use that for now. thanks

@pzi
Copy link
Author

pzi commented Nov 13, 2019

I was considering submitting a PR to change the following line:

const namedSet = (state: any, name?: any)

in https://github.com/react-spring/zustand/blob/master/src/middleware.ts#L33 to

const namedSet = (state: any, name: any = 'action')

but that would mean it would always log for to devtools for everyone. Unsure if that's desired?

It actually would also make more sense to remove the type from name as it will just infer the string type from the default action value.
So it could just be: const namedSet = (state: any, name = 'action')

@drcmda
Copy link
Member

drcmda commented Nov 13, 2019

@JeremyRH what do you think?

@JeremyRH
Copy link
Contributor

@pzi Your issue Expected 1 argument but got 2 is caused by this:

const store: StateCreator<DefaultState>

The StateCreator type shouldn't be used with the middleware's devtools function.
Try this:

type DevToolsStateSetter<T> = (partial: PartialState<T>, name?: string) => void

const store = (
  set: DevToolsStateSetter<DefaultState>,
  get: GetState<DefaultState>
): DefaultState => ({
  ...DefaultStoreState,
  ...
})

We should export a DevToolsStateCreator type in middleware to solve this. We should also default name to 'action' like you said.

@cassandramfrancis
Copy link

cassandramfrancis commented Nov 14, 2019

For anyone who comes here looking for a quick fix, my solution was to make a type based on the normal StateCreator:

import { State, SetState, StateCreator } from 'zustand';

/** To allow named functions when using devtools */
type StateCreatorDev<T extends State> = (
  set: (partial: Parameters<SetState<T>>[0], name?: string) => void,
  get: Parameters<StateCreator<T>>[1],
  api: Parameters<StateCreator<T>>[2]
) => T;

(edited to work properly, my test wasn't correct)

@unbiased-dev
Copy link

Can we get this in the docs?

@MillerGregor
Copy link

I followed the naming suggestions in this thread, but was still not seeing actions.
I found a quick fix... see PR #117

This was referenced Jun 29, 2020
@dai-shi
Copy link
Member

dai-shi commented Aug 15, 2020

Does anybody know what's the status of this issue?

@sanojsilva
Copy link

Hey @dai-shi,

@VuHG Solution works. But however, if we use immer we have to pass args like below otherwise it will not show state updates in the dev tools.

const immer = (config) => (set, get, api) =>
  config((fn, args) => {
	// args equals to the name we passed to the function in our store ("SET_CART_OPEN", "SET_BACKDROP_OPEN")
    return set(produce(fn), args), get, api;
  });

const [useStore] = create<Store>(
  devtools(
    immer((set) => ({
      cart: {
        open: false,
        setOpen: (value) =>
          set((state) => {
            if (typeof value !== "undefined") {
              state.cart.open = value;
            } else {
              state.cart.open = !state.cart.open;
            }
          }, "SET_CART_OPEN"),
      },
      backdrop: {
        open: false,
        setBackdropOpen: (value) => {
          set((state) => {
            state.backdrop.open = value;
          }, "SET_BACKDROP_OPEN");
        },
      },
    }))
  )
);

@dai-shi
Copy link
Member

dai-shi commented Aug 17, 2020

@sanojsilva Thanks for the note. Let me read the thread again.

So, the issue can be divided into three pieces, correct?

  1. namedSet should have default name
  2. TS type in devtools doesn't allow adding set names.
  3. Typing with immer middleware needs a workaround

BTW, I have some questions about typing with immer. #96

@dai-shi dai-shi added the bug Something isn't working label Aug 17, 2020
@dai-shi
Copy link
Member

dai-shi commented Aug 18, 2020

v3 has replace flag in the second argument. we need to find a better way for the devtools middleware. cc @drcmda

@dai-shi dai-shi changed the title devtools doesn't log actions since v2 [devtools] doesn't log actions since v2 Aug 19, 2020
@dai-shi
Copy link
Member

dai-shi commented Aug 19, 2020

Now v3 is out, We need to fix this. Does anyone have ideas?

@dai-shi dai-shi added the help wanted Please someone help on this label Aug 19, 2020
@WhiteCollarParker
Copy link

Very helpful issue thread! Saved me some hours

@dai-shi
Copy link
Member

dai-shi commented Aug 25, 2020

I think I fixed it fully. #167

@dai-shi dai-shi removed the help wanted Please someone help on this label Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants