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

TS issue with the set function when using devtools (with older TS versions) #1013

Closed
kwaimind opened this issue Jun 16, 2022 · 31 comments · Fixed by #1348
Closed

TS issue with the set function when using devtools (with older TS versions) #1013

kwaimind opened this issue Jun 16, 2022 · 31 comments · Fixed by #1348

Comments

@kwaimind
Copy link

Hey there zustand team

I'm having a problem when using zustand with devtools in my next.js project.

When running the store locally, everything works fine and I'm seeing devtools. Awesome.

But, when building my app for production with nx/next.js, it throws the following error:

Type error: Expected 3-4 arguments, but got 1.

  62 |       damageLocation: TDamageLocation
  63 |     ) => {
> 64 |       set({ damageType, damageLocation, view: 'station' });
     |       ^
  65 |     },
  66 |     updateStation: (station: IStationState) => {
  67 |       set({ station, view: 'timeSlot' });

What have I do so far:

  1. Tried to follow this How to use `devtools` middleware with typescript #976 and looked at the tests/middlewareTypes.test.tsx and tests/devtools.test.tsx examples but not successful.
  2. Tried to type the store like in the example above: create<InitialState, [["zustand/devtools", InitialState]]>
  3. Tried changing the set function called on line 64 (example above) but then I get the same error on a different function

Using the following core dependencies:

  • next: 12.1.0,
  • nx: 13.9.5,
  • react: 17.0.2,
  • zustand: ^4.0.0-rc.1

Let me know if you need more details or if this makes no sense.

@dai-shi
Copy link
Member

dai-shi commented Jun 16, 2022

Is it TypeScript issue? or runtime issue?

@dai-shi dai-shi added the help wanted Please someone help on this label Jun 16, 2022
@kwaimind
Copy link
Author

Is it TypeScript issue? or runtime issue?

Are you asking me? I think it is a TypeScript issue at build time.

@dai-shi
Copy link
Member

dai-shi commented Jun 16, 2022

Is it TypeScript issue? or runtime issue?

Are you asking me? I think it is a TypeScript issue at build time.

If it's a TS issue at build time, does it go away if you remove devtools?
(Then, enabled option might help.)

but, tbh, I'm not sure how TS error can only happen for production build.

@hasan-almujtaba
Copy link

I also have same issue in vscode, give set third argument give me "Expected 1-2 arguments, but got 3.ts(2554)" like in the screenshot.

image

@kwaimind
Copy link
Author

@dai-shi yes if we remove devtools from the create store setup then everything works as expected.

I can try and use the enabled flag and see what that does. Let me get back to you.

@dai-shi
Copy link
Member

dai-shi commented Jun 20, 2022

I also have same issue in vscode

Is this development mode? not production build?
Can anyone reproduce with codesandbox? or small repo?

"Expected 1-2 arguments, but got 3.ts(2554)"

Oh, it's a different error from the OP one.

@jenglamlow
Copy link
Contributor

jenglamlow commented Jun 20, 2022

I'm having the same issue as @hasan-almujtaba as well. But I don't get this issue when experimenting on small repo. I'm getting the error on other large project when trying to upgrade Zustand to v4.

The issue is Typescript is expecting the required third a1: undefined parameter, as shown as below

Screenshot 2022-06-20 at 10 31 48 PM

@hasan-almujtaba
Copy link

Thanks for fast response @dai-shi, i'm using your awesome library (Zustand) in this repo : https://github.com/hasan-Almujtaba/next-starter . It's looks like it just a TS typing problem because it's work fine in redux devtools.

@kwaimind
Copy link
Author

kwaimind commented Jun 20, 2022

@dai-shi Thanks for your help so far, really appreciate it.

So, I tried the enabled flag but that didn't work, and the build still fails.

Instead I tried to remove the parentheses directly after the create func (the opposite of what the TS docs say) and that gives me a new error (probably because I'm using zustand v4):

Argument of type 'StateCreator<IBookingBarState, [], [["zustand/devtools", never]], IBookingBarState>' is not assignable to parameter of type 'StateCreator<IBookingBarState, [], [], IBookingBarState>'.
  Type 'StateCreator<IBookingBarState, [], [["zustand/devtools", never]], IBookingBarState>' is not assignable to type '{ $$storeMutators?: [] | undefined; }'.
    Types of property '$$storeMutators' are incompatible.
      Type '[["zustand/devtools", never]] | undefined' is not assignable to type '[] | undefined'.
        Type '[["zustand/devtools", never]]' is not assignable to type '[]'.
          Source has 1 element(s) but target allows only 0.ts(2345)

So then I tried to use this solution and gave some extra generic types like this: create<IBookingBarState, [['zustand/devtools', IBookingBarState]]> but that gives me another new error at build time only:

Type error: Expression produces a union type that is too complex to represent.

  50 |   [['zustand/devtools', IBookingBarState]]
  51 | >(
> 52 |   devtools(
     |   ^
  53 |     (set) => ({
  54 |       view: null,
  55 |       damageType: null,

The setup I'm using is a Next.js app running inside an NX monorepo and so it is the Next.js build that is failing at build time. Running in dev mode works 100% of the time.

I made a quick demo that seems to reproduce the same problem and as @hasan-almujtaba suggests, it might just be the typings are missing some partials.

https://stackblitz.com/edit/nextjs-wjmkub?file=pages%2Findex.tsx

Using set with just a callback throws the same error: Expected 3-4 arguments, but got 1.(2554), but adding the replace and a1 args seem to fix the issue.

Hope this helps you investigate the issue.

@dai-shi
Copy link
Member

dai-shi commented Jun 20, 2022

Okay, I think I assumed the issue totally wrong. I thought it's only production build issue, but it seems just a TS typing issue.

I don't know where a1 comes from and why replace is not optional.

https://codesandbox.io/s/wild-browser-bj8x83?file=/src/App.tsx works in csb. (@hasan-almujtaba your issue seems different, please try to have your function inline.)

@devanshj Can you please help this?

@hasan-almujtaba
Copy link

@dai-shi it's works fine if i have my function inline, but it's give me error when i'm trying to split my store to separate slices. I think the problem here is in the SetState type. i import it in the first line in https://github.com/hasan-almujtaba/next-starter/blob/main/store/example.ts and use it in line 4. I'm following this typescript example for splitting slice to separate files https://codesandbox.io/s/nostalgic-voice-3knvd?file=/src/App.js.

Also if i look to https://github.com/pmndrs/zustand/blob/main/src/vanilla.ts on line 16 - 20, there's only 2 parameter defined where actually you need third parameter for defining actionType name.

@dai-shi
Copy link
Member

dai-shi commented Jun 21, 2022

@hasan-almujtaba Would you please open a new discussion? Or maybe add comments in #976? #980 is also related.


This issue focuses on the problem even with inline functions.
image

@WaleedZ90
Copy link

It's a typescript issue, the type SetState doesn't have the actionName defined in it's type definition.
The work around that I've is extend the state myself.

type SetState<T extends State> = {
  _(partial: T | Partial<T> | ((state: T) => T | Partial<T>), replace?: boolean | undefined, actionName?: string): void;
}['_'];

@dai-shi
Copy link
Member

dai-shi commented Jun 21, 2022

This issue focuses on the problem even with inline functions.

Please focus on that ☝️ in this issue.


See also #980.

This is intentional because vanilla store doesn't know about the third parameter. Only devtools middleware has the idea and extends the type.

You can comment in #980.

@devanshj
Copy link
Contributor

devanshj commented Jun 21, 2022

The issue is reproducible in this stackblitz as said above, but not reproducible in this typescript playground with the same code. That means zustand types are correct, it's NextJS that is doing something funny. Maybe the tsconfig.json is the culprit, maybe those triple slash directives in next-env.d.ts are the culprit, maybe something else, we don't know.

So the way you'd debug is the way you debug all bugs—by binary search. What you'd want to do is start with a fresh folder, install typescript and zustand, and use tsc --init to generate the tsconfig.json and copy the code in the playground, it should compile. Now from here, start adding stuff and making changes to resemble the NextJS reproduction. In this process you'd find addition of what made the code not compile, for example you could find changing some field of the tsconfig.json makes the code no longer compile. Report your findings here.

If anyone here can't do this I'll do it if and when I have the time for it. But one thing that is 99% certain is that there's no problem in zustand types.

@kwaimind
Copy link
Author

kwaimind commented Jun 27, 2022

@devanshj thanks for the help ❤️ I found the issue and it was our TS version. Seems the version 4.6.4 and above works (your demo was using the latest stable 4.7.4) but our repo was using 4.3.5.

Downgrading your demo caused the same issue. Updating our TS dependency solves the issue.

@devanshj
Copy link
Contributor

devanshj commented Jun 28, 2022

Ah I see, glad I was of help!

It would be nice if we could document the minimum ts version required. Perhaps someone could confirm it by writing a script that might look something like this (perhaps a bash version of it)...

import { $ } from "zx"

let v = 4.7
while (v >= 2.0) {
  await $`yarn install typescript@${v}`
  try {
    await $`yarn tsc --noEmit`
    console.log(`${v} compiles`)
  } catch {
    console.log(`${v} doesn't compile`)
    console.log(`---`)
    console.log(`Minimum version requirement is ${v + 0.1}`)
    break
  }
  v -= 0.1
}

One could even write a more sophisticated script that also retries compiling after downleveling (with downlevel-dts) to the current version each time it doesn't compile, with that we could perhaps get a lower requirement.

@dai-shi
Copy link
Member

dai-shi commented Jun 28, 2022

So, we don't use the new features of TS, but we rely on behaviors of new TS. It's unfortunate that we can't support old TS (as zustand is trying to be "stable" compared to the other two), but can't be helped.

@devanshj
Copy link
Contributor

devanshj commented Jun 28, 2022

Yeah although I'm quite surprised that the types are leveraging behaviors that are so recent that even something like 4.3 doesn't compile. Usually it's not a big deal to update typescript to latest version because typescript has a goal of being mostly non-breaking—more strictness and consequent breakage is opt-in via flags.

I think the reason that old versions don't compile could also be rather stupid like old versions not being able to recognise named tuples (ie [foo: number] instead of [number]) hence it's good to run the above script with added downleveling.

@dai-shi
Copy link
Member

dai-shi commented Jun 28, 2022

I'm quite surprised that the types are leveraging behaviors that are so recent that even something like 4.3 doesn't compile.

Haha, if you didn't know, we didn't know.

Usually it's not a big deal to update typescript to latest version because typescript has a goal of being mostly non-breaking

You are too optimistic about it. TypeScript package can be installed by a framework, and (beginner) devs don't even touch it.

Anyway, I agree that we should know the minimum requirement (with/out downlevel-dts). Would anyone be willing to help?

@dai-shi dai-shi changed the title TS issue with the set function when using devtools TS issue with the set function when using devtools (with older TS versions) Jun 28, 2022
@dai-shi
Copy link
Member

dai-shi commented Jun 28, 2022

I think the reason that old versions don't compile

@devanshj My gut feeling is TakeTwo.

type TakeTwo<T> = T extends []
? [undefined, undefined]
: T extends [unknown]
? [...a0: T, a1: undefined]
: T extends [unknown?]
? [...a0: T, a1: undefined]
: T extends [unknown, unknown]
? T
: T extends [unknown, unknown?]
? T
: T extends [unknown?, unknown?]
? T
: T extends [infer A0, infer A1, ...unknown[]]
? [A0, A1]
: T extends [infer A0, (infer A1)?, ...unknown[]]
? [A0, A1?]
: T extends [(infer A0)?, (infer A1)?, ...unknown[]]
? [A0?, A1?]
: never

Yeah, this is where a1 comes from.

If this is the only blocker, we might be able to consider changing JS implementation to avoid TakeTwo. (Can we do (...args, nameOrAction) => { ??)

@devanshj
Copy link
Contributor

TypeScript package can be installed by a framework, and (beginner) devs don't even touch it.

I get that but what I meant was once the user knows old version is the problem, all they have to do is run yarn add typescript@latest, there is no migration or extra steps needed whatsoever (thanks to TypeScript being non-breaking). But of course, it's difficult to tell by the error alone that the old version is the problem.

If this is the only blocker, we might be able to consider changing JS implementation to avoid TakeTwo. (Can we do (...args, nameOrAction) => { ??)

We could do that in that case we won't need TakeTwo—instead of writing [...TakeTwo<A>, string] we would write [...A, string].

But I think we should try to solve it starting from the most direct solution—downleveling. Because today it's TakeTwo, tomorrow it could be something else. So the first we should see if downleveling resolves the issue, if it doesn't then we should try to simplify the type by trading-off some unused correctness, and then if that too doesn't work then we can change the js implementation.

@naps62
Copy link

naps62 commented Jul 9, 2022

I'm running into the same error, but I'm using typescript 4.7.4, which according to a comment above, should work?

image

my tsconfig:

{
  "compilerOptions": {
    "target": "es5",
    "lib": ["dom", "dom.iterable", "esnext"],
    "allowJs": true,
    "skipLibCheck": true,
    "strict": true,
    "forceConsistentCasingInFileNames": true,
    "noEmit": true,
    "esModuleInterop": true,
    "module": "esnext",
    "moduleResolution": "node",
    "resolveJsonModule": true,
    "isolatedModules": true,
    "jsx": "preserve",
    "incremental": true,
    "noUncheckedIndexedAccess": true
  },
  "include": ["next-env.d.ts", "**/*.ts", "**/*.tsx"],
  "exclude": ["node_modules"]
}

@JClackett
Copy link

Have the same issue as @naps62 after upgrading to v4,

type StoredPages = { pages: RecentPage[]; setPages: (pages: RecentPage[]) => void }

export const useStoredPages = create<StoredPages>(
  persist(
    (set) => ({
      pages: [],
      setPages: (pages: RecentPage[]) => set(() => ({ pages })),
    }),
    { name: RECENT_PAGES_KEY},
  ),
)
Argument of type 'StateCreator<StoredPages, [], [["zustand/persist", StoredPages]], StoredPages>' is not assignable to parameter of type 'StateCreator<StoredPages, [], [], StoredPages>'.
  Type 'StateCreator<StoredPages, [], [["zustand/persist", StoredPages]], StoredPages>' is not assignable to type '{ $$storeMutators?: [] | undefined; }'.
    Types of property '$$storeMutators' are incompatible.
      Type '[["zustand/persist", StoredPages]] | undefined' is not assignable to type '[] | undefined'.
        Type '[["zustand/persist", StoredPages]]' is not assignable to type '[]'.
          Source has 1 element(s) but target allows only 0.ts(2345)

@dai-shi
Copy link
Member

dai-shi commented Aug 3, 2022

Are you sure using TypeScript 4.7.4?

@naps62
Copy link

naps62 commented Aug 3, 2022

@JClackett forgot to update here, but after some more searching I found the problem on another issue here that I can't recall

you should actually do

const store = create<StoredPages>()(
  // ...
)

notice the extra parenthesis

@JClackett
Copy link

@JClackett forgot to update here, but after some more searching I found the problem on another issue here that I can't recall

you should actually do

const store = create<StoredPages>()(

  // ...

)

notice the extra parenthesis

Nice! Thanks! - functionally still works without the extra parenthesis? Is that right

@dai-shi
Copy link
Member

dai-shi commented Sep 2, 2022

As https://tsplay.dev/N5LjVw shows, TS v4.4.4 causes an error.
TS v4.5.5 works fine.
Maybe, we should note about minimal supported TS version. @chrisk-7777
(I think if one doesn't use middleware, TS v4.4.4 works fine, though.)

@rptrainor
Copy link

Yes this resolved the problem for me too:

export const useStoreState = create()(
persist(
(set) => ({
data: undefined,
error: false,
isLoading: false,
setData: (data) => set((state) => ({ ...state, data })),
setError: (error) => set((state) => ({ ...state, error })),
setIsLoading: (isLoading) => set((state) => ({ ...state, isLoading })),
}),
{
name: 'local_storage_key_here',
}
)
);

@cyango
Copy link

cyango commented Jun 25, 2023

Why is this not in the documentation? Or am I missing it?

@dai-shi
Copy link
Member

dai-shi commented Jun 25, 2023

I think #1348 solved the original issue. If you still have some issue, please open a new discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
10 participants