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

Trying to migrate to v4 but hitting issue with mocking #1059

Closed
httpete opened this issue Jul 5, 2022 · 24 comments
Closed

Trying to migrate to v4 but hitting issue with mocking #1059

httpete opened this issue Jul 5, 2022 · 24 comments
Labels
documentation Improvements or additions to documentation help wanted Please someone help on this

Comments

@httpete
Copy link

httpete commented Jul 5, 2022

I was using the mocks/zustand.ts that contained the boilerplate from the doc, but now I get
TypeError: store.getState is not a function

import { act } from 'react-dom/test-utils';
import actualCreate from 'zustand';

// a variable to hold reset functions for all stores declared in the app
const storeResetFns = new Set();

// when creating a store, we get its initial state, create a reset function and add it in the set
const create = createState => {
  const store = actualCreate(createState);
  const initialState = store().getState();
  storeResetFns.add(() => store.setState(initialState, true));

  return store;
};

// Reset all stores after each test run
afterEach(() => {
  act(() => storeResetFns.forEach(resetFn => resetFn()));
});

export default create;
@dai-shi
Copy link
Member

dai-shi commented Jul 5, 2022

Hm, not sure, but does this help?

const store = actualCreate(createState || () => ({}));

@devanshj
Copy link
Contributor

devanshj commented Jul 6, 2022

Guessing but you'll probably have to change your mocked create to allow the curried usage...

const create = f =>
  f === undefined ? createImpl : createImpl(f)

const createImpl = createState => {
  const store = actualCreate(createState)
  const initialState = store().getState()
  storeResetFns.add(() => store.setState(initialState, true))

  return store
}

@httpete
Copy link
Author

httpete commented Jul 7, 2022

OK Ill try soon thank you. I absolutely love the simplicity of Zustand 3, but I am finding that Zustand 4 just seems to be harder to use and I am not sure of the benefit. I really like to keep up to date, but my first shot at migrating us to 4rc1 didn't go too well. I will try again. When do you foresee a final 4?

@dai-shi
Copy link
Member

dai-shi commented Jul 7, 2022

When do you foresee a final 4?

I will merge #953, #1029, #1051, when they are ready, and release rc.2.
If we don't get any issues with it, we will finalize it.

@httpete
Copy link
Author

httpete commented Jul 7, 2022

Thank you @dai-shi I will try right away and provide feedback.

@devanshj
Copy link
Contributor

devanshj commented Jul 8, 2022

I am not sure of the benefit.

You can see the diff of middlewareTypes.test.tsx between v3.6.9 and v4. Here's an excerpt...

Before (v3.6.9):

const useStore = create<
  CounterState,
  SetState<CounterState>,
  GetState<CounterState>,
  StoreApiWithSubscribeWithSelector<CounterState> &
    StoreApiWithPersist<CounterState> &
    StoreApiWithDevtools<CounterState>
>(
  devtools(
    subscribeWithSelector(
      persist(
        (set, get) => ({
          count: 0,
          inc: () => set({ count: get().count + 1 }, false),
        }),
        { name: 'count' }
      )
    ),
    { name: 'prefix' }
  )
)

After (v4.0.0-rc1):

const useStore = create<CounterState>()(
  devtools(
    subscribeWithSelector(
      persist(
        (set, get) => ({
          count: 0,
          inc: () => set({ count: get().count + 1 }, false),
        }),
        { name: 'count' }
      )
    ),
    { name: 'prefix' }
  )
)

my first shot at migrating us to 4rc1 didn't go too well.

Btw there's a migration guide if you missed it. If you follow it you will have mostly no problems. And there is no breaking change in the runtime code whatsoever.

Also looks like you were not using TypeScript for your mocking code or else the bug would have been caught. For instance you were writing create()(...) ie passing undefined to create but not handling it.

I will merge #953, #1029, #1051, when they are ready, and release rc.2.

Let's also wait for my PR to fix #1046, I'll open soon.

@dai-shi
Copy link
Member

dai-shi commented Jul 16, 2022

Let's also wait for my PR to fix #1046, I'll open soon.

@devanshj Is it coming soon? Everything else is ready, so I can release rc.2.

@devanshj
Copy link
Contributor

The PR was for option 2, but now that we went with option 1, we don't need it and #1090 suffices.

@dai-shi
Copy link
Member

dai-shi commented Jul 17, 2022

Cool. Thanks.

@httpete
Copy link
Author

httpete commented Jul 23, 2022

Thanks all, as long as the docs that have that snip of "how to mock zustand" is updated when you release, I don't think others will be tripped on this.

https://github.com/pmndrs/zustand/wiki/Testing

@dai-shi
Copy link
Member

dai-shi commented Jul 23, 2022

We want to create new testing guide #968, but it's stalled. It will not be done soon.
Could you update the wiki page as much as you can?

@dai-shi dai-shi added help wanted Please someone help on this documentation Improvements or additions to documentation labels Jul 24, 2022
@httpete
Copy link
Author

httpete commented Jul 27, 2022

Hm, not sure, but does this help?

const store = actualCreate(createState || () => ({}));

It doesn't. I am finding now that Zustand 4 works perfectly in the browser, but my zustand mock isn't resetting properly. I am trying, with the ideas from @devanshj to accommodate the currying but I haven't found it yet.

@httpete
Copy link
Author

httpete commented Jul 28, 2022

I had to give up, I actually determined that I don't need (or want) because all of our components are wrapped in a unique ZustandContext and instantiated fresh with each test. I don't have the problem of a global state that needs resetting. So I am punting.

I reviewed the testing guide, but as you say it is raw. The barest min would at least have the Testing Wiki have a starter snip with something that works with Z4.

@arvinxx
Copy link
Contributor

arvinxx commented Aug 7, 2022

@dai-shi I try to mirgration from v3 to v4, but failed too…

How to fix the curried usage?

const create = f =>
  f === undefined ? createImpl : createImpl(f)

const createImpl = createState => {
  const store = actualCreate(createState)
  const initialState = store().getState()
  storeResetFns.add(() => store.setState(initialState, true))

  return store
}

I have tried this but didn't work.

@dai-shi
Copy link
Member

dai-shi commented Aug 7, 2022

How to fix the curried usage?

Hm, it has to be something like this?

const create = () => createState => {
  const store = actualCreate(createState)
  const initialState = store.getState()
  storeResetFns.add(() => store.setState(initialState, true))
  return store
}

I reviewed the testing guide, but as you say it is raw. The barest min would at least have the Testing Wiki have a starter snip with something that works with Z4.

We need someone to help fixing the doc.

@arvinxx
Copy link
Contributor

arvinxx commented Aug 15, 2022

How to fix the curried usage?

Hm, it has to be something like this?

const create = () => createState => {
  const store = actualCreate(createState)
  const initialState = store.getState()
  storeResetFns.add(() => store.setState(initialState, true))
  return store
}

I reviewed the testing guide, but as you say it is raw. The barest min would at least have the Testing Wiki have a starter snip with something that works with Z4.

We need someone to help fixing the doc.

don't work with createContext usage.

@dai-shi
Copy link
Member

dai-shi commented Aug 17, 2022

don't work with createContext usage.

I'm not familiar with testing with React context, but do people inject store with context, don't they?

@razzeee
Copy link

razzeee commented Aug 29, 2022

I'm getting the same error as @httpete when using import actualCreate from 'zustand' inspite of us using jest.

If I use const actualCreate = jest.requireActual('zustand') // if using jest like the docs mention I end up with TypeError: actualCreate is not a function

@gfox1984
Copy link

gfox1984 commented Aug 30, 2022

I had the same error as @httpete and @razzeee, but with the change @dai-shi suggested in his previous comment, it's working fine now.

Im using Jest and my file is called zustand.ts (not .js, unlike the docs suggest) and it is located in the __mocks__ folder (at the root of my tests folder, where I've put all my tests). The content is:

import { act } from 'react-dom/test-utils';
import actualCreate from 'zustand';

// a variable to hold reset functions for all stores declared in the app
const storeResetFns = new Set<() => void>();

// when creating a store, we get its initial state, create a reset function and add it in the set
const create = () => (createState) => {
  const store = actualCreate(createState);
  const initialState = store.getState();
  storeResetFns.add(() => store.setState(initialState, true));
  return store;
};

// Reset all stores after each test run
beforeEach(() => {
  act(() => storeResetFns.forEach((resetFn) => resetFn()));
});

export default create;

I think the docs need an update...

EDIT: I also changed Set to Set<() => void> to avoid typescript error when using resetFn.

@arvinxx
Copy link
Contributor

arvinxx commented Sep 9, 2022

How to fix the curried usage?

Hm, it has to be something like this?

const create = () => createState => {
  const store = actualCreate(createState)
  const initialState = store.getState()
  storeResetFns.add(() => store.setState(initialState, true))
  return store
}

I reviewed the testing guide, but as you say it is raw. The barest min would at least have the Testing Wiki have a starter snip with something that works with Z4.

We need someone to help fixing the doc.

don't work with createContext usage.

I have fix the test by export useStore

import actualCreate from 'zustand';
import { act } from '@testing-library/react-hooks';

// a variable to hold reset functions for all stores declared in the app
const storeResetFns = new Set();

// when creating a store, we get its initial state, create a reset function and add it in the set
-const create = (createState) => {
+const createImpl = (createState) => {
  console.log(createState);
  const store = actualCreate(createState);
  const initialState = store.getState();
  storeResetFns.add(() => store.setState(initialState, true));
  return store;
};

// Reset all stores after each test run
afterEach(() => {
  act(() => storeResetFns.forEach((resetFn) => resetFn()));
});

+ const create = (f) => (f === undefined ? createImpl : createImpl(f));

export default create;

+ export { useStore } from 'zustand';

@tfreitas90
Copy link

tfreitas90 commented Sep 27, 2022

Hey, I had the same problem than OP and I fixed with the following:
The difference is, instead of
const create = createState => {...} it should be:
const create = () => createState => {...}.

How to fix the curried usage?

Hm, it has to be something like this?

const create = () => createState => {
  const store = actualCreate(createState)
  const initialState = store.getState()
  storeResetFns.add(() => store.setState(initialState, true))
  return store
}

I reviewed the testing guide, but as you say it is raw. The barest min would at least have the Testing Wiki have a starter snip with something that works with Z4.

We need someone to help fixing the doc.

Thanks!

@kandrelczyk
Copy link

Hi, hit the same issue and resolved it by migrating my project to jotai.

@sewera
Copy link
Collaborator

sewera commented Feb 16, 2023

I believe this issue is resolved now with const create = () => createState => {...}, as per this comment, so I'll close it now.

For reference, I'll put the relevant docs links:

If a similar case happens in the future, this issue can be re-opened or, better yet, referenced.

@sewera sewera closed this as completed Feb 16, 2023
@tom2strobl
Copy link

I know this is an older+closed issue, but for others struggling to get mock-resets to work properly I still want to mention that what @devanshj said in #1059 (comment) was necessary for us since we use middleware and sometimes the curried, sometimes the regular invocation.

We're using ts/jest/TLR and saw great results like so:

import { act } from '@testing-library/react'
import { create as createType, StateCreator } from 'zustand'

const zustand = jest.requireActual('zustand')
const actualCreate: typeof createType = zustand.create

// a variable to hold reset functions for all stores declared in the app
const storeResetFns = new Set<() => void>()

// when creating a store, we get its initial state, create a reset function and add it in the set
const createImpl = <S>(createState: StateCreator<S>) => {
  const store = actualCreate<S>(createState)
  const initialState = store.getState()
  storeResetFns.add(() => store.setState(initialState, true))
  return store
}

// support currying
export function create<S>(f: StateCreator<S>) {
  return f === undefined ? createImpl : createImpl(f)
}

// Reset all stores after each test run
beforeEach(() => {
  act(() => storeResetFns.forEach((resetFn) => resetFn()))
})

Note that the docs still tell users to only support the uncurried version, which might cause trouble for some (like us).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation help wanted Please someone help on this
Projects
None yet
Development

No branches or pull requests

10 participants