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

docs: testing for using typescript #1303

Merged
merged 1 commit into from
Sep 21, 2022
Merged

Conversation

mugi-uno
Copy link
Contributor

Related Issues

(None)

Summary

Add an explanation of how to mock in TypeScript to the Testing doc.
(If follow the TypeScript Guide, create is executed in a different way.)

Check List

  • yarn run prettier for formatting code and docs

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 21, 2022

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 f08f316:

Sandbox Source
React Configuration
React Typescript Configuration
React Browserify Configuration
React Snowpack Configuration
React Parcel Configuration
Next.js Configuration

Copy link
Collaborator

@chrisk-7777 chrisk-7777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mugi-uno !
I just had a few comments/questions 👍

@@ -38,6 +38,34 @@ The way you mock a dependency depends on your test runner/library.

In [jest](https://jestjs.io/), you can create a `__mocks__/zustand.js` and place the code in that file. If your app is using `zustand/vanilla` instead of `zustand`, then you'll have to place the above code in `__mocks__/zustand/vanilla.js`.

### TypeScript Usage
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we change this to sentence case? "TypeScript usage"
(I'm slowly trying to get this consistent across the docs)

@@ -38,6 +38,34 @@ The way you mock a dependency depends on your test runner/library.

In [jest](https://jestjs.io/), you can create a `__mocks__/zustand.js` and place the code in that file. If your app is using `zustand/vanilla` instead of `zustand`, then you'll have to place the above code in `__mocks__/zustand/vanilla.js`.

### TypeScript Usage

If you are using zustand according to [TypeScript Guide](./typescript.md), use the following code.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you be ok with a slight tweak here? I'm thinking something like:

If you are using zustand, as documented in [TypeScript Guide](./typescript.md), use the following code:

@chrisk-7777
Copy link
Collaborator

chrisk-7777 commented Sep 21, 2022

I'm just testing this locally, as well as reading through this thread (#1059), and I think the testing doc in general needs a pretty big uplift.

Edit: The above doesn't need to be addressed in this PR, just a note.

@dai-shi
Copy link
Member

dai-shi commented Sep 21, 2022

For reference: #271

})

export default create
```
Copy link
Collaborator

@chrisk-7777 chrisk-7777 Sep 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get type errors locally for this snippet.

I believe something like this would work:

import actualCreate, { StateCreator } from 'zustand'
// const actualCreate = jest.requireActual('zustand') // if using jest
import { act } from 'react-dom/test-utils'

// 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 = () => <S>(createState: StateCreator<S>) => {
  const store = actualCreate<S>(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()))
})

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not able to take into account the type.
I tried it and it worked correctly, including type checking 👌

@chrisk-7777
Copy link
Collaborator

Thanks dai-shi. I'll read through that today and try to come up with a consolidated testing page over the next few days or so

@mugi-uno mugi-uno force-pushed the fix/testing-doc branch 3 times, most recently from ed8261b to 0539b00 Compare September 21, 2022 04:57
@mugi-uno
Copy link
Contributor Author

@chrisk-7777
Thanks for review.
I fixed all.
f08f316

Copy link
Collaborator

@chrisk-7777 chrisk-7777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for looking at the feedback

@dai-shi dai-shi merged commit 77d14b1 into pmndrs:main Sep 21, 2022
@mugi-uno mugi-uno deleted the fix/testing-doc branch September 21, 2022 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants