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

Update how to reset state doc #1495

Merged
merged 6 commits into from Dec 27, 2022

Conversation

dbritto-dev
Copy link
Collaborator

Related Issues

Fixes #1494

Summary

  • Update how to reset state doc
  • Update codesandbox advance demo
  • Add codesandbox immer demo

Check List

  • yarn run prettier for formatting code and docs

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 23, 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 4816067:

Sandbox Source
React Configuration
React Typescript Configuration
React Browserify Configuration
React Snowpack Configuration
React Parcel Configuration
Next.js Configuration
zustand-with-immer-reset-all-stores Issue #1494

@dbritto-dev
Copy link
Collaborator Author

@dai-shi @KevinMusgrave here's the PR :)

@dbritto-dev
Copy link
Collaborator Author

@chrisk-7777 would you mind reviewing this PR?

@dai-shi
Copy link
Member

dai-shi commented Dec 24, 2022

Let's have @devanshj eyes on this, in case there are better solutions.

@cuppachino
Copy link
Contributor

Should the curry workaround have a mention? It allows multiple middleware.

// Use a trailing comma after `T` in tsx files.
const create = <T,>() => <
  Mos extends [StoreMutatorIdentifier, unknown][] = []
>(
  initializer: StateCreator<T, [], Mos>
) => {
  const slice = _create(initializer);
  const initialSlice = slice.getState();
  resetters.push(() => {
    slice.setState(initialSlice, true);
  });

  return slice;
};
const useSalmon = create<StateSalmon & ActionsSalmon>()(
  immer(
    devtools((set, get) => ({
      // ...
    }))
  )
);

@dbritto-dev
Copy link
Collaborator Author

Should the curry workaround have a mention? It allows multiple middleware.

// Use a trailing comma after `T` in tsx files.
const create = <T,>() => <
  Mos extends [StoreMutatorIdentifier, unknown][] = []
>(
  initializer: StateCreator<T, [], Mos>
) => {
  const slice = _create(initializer);
  const initialSlice = slice.getState();
  resetters.push(() => {
    slice.setState(initialSlice, true);
  });

  return slice;
};
const useSalmon = create<StateSalmon & ActionsSalmon>()(
  immer(
    devtools((set, get) => ({
      // ...
    }))
  )
);

Do you know if there's a way to avoid that formatting tools, like prettier, remove that trailing comma?

@cuppachino
Copy link
Contributor

cuppachino commented Dec 24, 2022

@dbritto-dev here's a couple ideas:

Option 1.

Use .ts extension in markdown and encourage users to separate stores from components.

- ```tsx
+ ```ts

Option 2.

Tell prettier to skip formatting create

+ /* prettier-ignore */
const create = <T,>() => <

Option 3.

Type T as unknown (I feel like this hasn't worked for me in the past, but it seems to now).

- const create = <T,>() => <
+ const create = <T extends unknown> => <

Option 4.

Tell eslint we know _ does nothing.

+ /* eslint-disable @typescript-eslint/no-unused-vars */
- const create = <T,>() => <
+ const create = <T,_>() => <

Option 5.

Use a function expression (maybe not so consistent with other doc pages). This would mimic the declaration file for Create, which looks like an overloaded function given a variable name with const (without overloads, thoughts below).

- const create = <T,>() => <
-   Mos extends [StoreMutatorIdentifier, unknown][] = []
- >(
+  const create = function <T>() {
+   return <Mos extends [StoreMutatorIdentifier, unknown][] = []>(
    initializer: StateCreator<T, [], Mos>
  ) => {
    const slice = _create(initializer);
    const initialSlice = slice.getState();
    resetters.push(() => {
      slice.setState(initialSlice, true);
    });

    return slice;
+   };
};

Option 6.

Pull up the Mos parameter's default type

- const create = <T,>() => <
-   Mos extends [StoreMutatorIdentifier, unknown][] = []
+ const create = <T, _Mos extends [StoreMutatorIdentifier, unknown][] = []>() => <
+   Mos extends [StoreMutatorIdentifier, unknown][] = _Mos

Imo we shouldn't try to replicate the default export _create behavior because it's really a TS issue. If you need type inference and you're using typescript, use the curried version until TS gets smarter, otherwise, the issue isn't even an issue, right? The curried form should always be a function that returns a function, and it should be a separate example, if included at all/better solution isn't found.

TLDR: use option 6, don't overload.

@dai-shi
Copy link
Member

dai-shi commented Dec 24, 2022

To make prettier happy without changing configs, I prefer option 3. It means T extends unknown, right?

@cuppachino
Copy link
Contributor

cuppachino commented Dec 24, 2022

To make prettier happy without changing configs, I prefer option 3. It means T extends unknown, right?

Yeah, sorry about the typo.

Can you double check? There was also typo where I said option "D"... and all the options are numbered. I also added option 6, but the type of _Mos isn't actually enforced.

@dai-shi
Copy link
Member

dai-shi commented Dec 24, 2022

I mean this.

- const create = <T,>() => <
+ const create = <T extends unknown> => <

@dai-shi
Copy link
Member

dai-shi commented Dec 24, 2022

Do you know if there's a way to avoid that formatting tools, like prettier, remove that trailing comma?

Does changing "ts" to "tsx" after three backquotes help?

ref: prettier/prettier#6115

@cuppachino
Copy link
Contributor

Do you know if there's a way to avoid that formatting tools, like prettier, remove that trailing comma?

Does changing "ts" to "tsx" after three backquotes help?

ref: prettier/prettier#6115

Yes, but anyone who implements it in a tsx file will have the problem if they're also using prettier. The codesandboxes linked in #1494 autoformat on save, breaking the example.

@dbritto-dev
Copy link
Collaborator Author

dbritto-dev commented Dec 24, 2022

Changing the ts to tsx actually does nothing because tsx is just an alias of ts on blockquotes, has the same behavior

Copy link
Contributor

@devanshj devanshj left a comment

Choose a reason for hiding this comment

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

The type of create here has the same type as the original create, so there's no need of retyping it (the retyping is also incorrect as it misses the Mis type parameter), so one should just use typeof originalCreate. And it should also implement the curried usage.

So the final code would look something like this... (feel free to change formating, variable names, etc)

import _create, { StateCreator } from 'zustand'

const resetters: (() => void)[] =
  []

export const create = (<T extends unknown>(f: StateCreator<T> | undefined) => {
  if (f === undefined) return create
  const store = _create(f)
  const initialState = store.getState()
  resetters.push(() => {
    store.setState(initialState, true)
  })
  return store
}) as typeof _create

export const resetAllSlices = () => {
  for (const resetter of resetters) {
    resetter()
  }
}

Also the code snippet above this one should use the curried version ie create<State & Actions>()(...) instead of create<State & Actions>(...). The latter is not anywhere in the TypeScript docs (except for "create without curried workaround" section) and is not recommended.

@dbritto-dev
Copy link
Collaborator Author

The type of create here has the same type as the original create, so there's no need of retyping it (the retyping is also incorrect as it misses the Mis type parameter), so one should just use typeof originalCreate. And it should also implement the curried usage.

So the final code would look something like this... (feel free to change formating, variable names, etc)

import _create, { StateCreator } from 'zustand'

const resetters: (() => void)[] =
  []

export const create = (<T extends unknown>(f: StateCreator<T> | undefined) => {
  if (f === undefined) return create
  const store = _create(f)
  const initialState = store.getState()
  resetters.push(() => {
    store.setState(initialState, true)
  })
  return store
}) as typeof _create

export const resetAllSlices = () => {
  for (const resetter of resetters) {
    resetter()
  }
}

Also the code snippet above this one should use the curried version ie create<State & Actions>()(...) instead of create<State & Actions>(...). The latter is not anywhere in the TypeScript docs (except for "create without curried workaround" section) and is not recommended.

Thanks I'll apply the changes

@dbritto-dev
Copy link
Collaborator Author

@devanshj the changes are done :D

Copy link
Contributor

@devanshj devanshj left a comment

Choose a reason for hiding this comment

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

Left an addtional suggestion as a comment.

And let's also change create<State & Actions>(... into create<State & Action>()(... in the first snippet of the document as I suggested earlier (can't comment on that line as it's not part of the change).

Other than these changes it looks good.

docs/guides/how-to-reset-state.md Show resolved Hide resolved
@dbritto-dev
Copy link
Collaborator Author

dbritto-dev commented Dec 25, 2022

Left an addtional suggestion as a comment.

And let's also change create<State & Actions>(... into create<State & Action>()(... in the first snippet of the document as I suggested earlier (can't comment on that line as it's not part of the change).

Other than these changes it looks good.

@devanshj the changes are done

@dbritto-dev dbritto-dev reopened this Dec 25, 2022
@devanshj
Copy link
Contributor

This one is still missing:

And let's also change create<State & Actions>(... into create<State & Action>()(... in the first snippet of the document as I suggested earlier (can't comment on that line as it's not part of the change).

@dbritto-dev
Copy link
Collaborator Author

This one is still missing:

And let's also change create<State & Actions>(... into create<State & Action>()(... in the first snippet of the document as I suggested earlier (can't comment on that line as it's not part of the change).

yeah, I updated the first codesandbox demo as well. Now we are good.

@devanshj
Copy link
Contributor

No I'm talking about line 30 in how-to-reset-state.md, that still needs a change, let me know if it's still not clear.

@dbritto-dev
Copy link
Collaborator Author

dbritto-dev commented Dec 26, 2022

No I'm talking about line 30 in how-to-reset-state.md, that still needs a change, let me know if it's still not clear.

I see. I thought you were talking about the codesandbox demos haha, my bad

@dbritto-dev
Copy link
Collaborator Author

@devanshj all yours

@dbritto-dev
Copy link
Collaborator Author

@dai-shi all yours :D

@dai-shi dai-shi merged commit b885055 into pmndrs:main Dec 27, 2022
@dbritto-dev dbritto-dev deleted the fix/how-to-reset-state-docs branch December 28, 2022 02:33
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.

Typescript Error when using Immer with custom "Reset Multiple Stores" function
4 participants