useShallow introduces dependency on react

[birtles]

With the introduction of useShallow, using zustand/shallow now introduces a dependency on 'react'.

For me that's problematic because I'm using Preact. Normally I can easily alias 'react' to preact/compat in my webpack config etc. but when using vitest there seems to be no way to alias dependencies inside node_modules (there are aliases in the test config but they don't apply inside node_modules). See vitest-dev/vitest#1652 (comment)

Effectively this means I can't test any code using zustand/shallow from vitest without installing React in addition to Preact.

Would it make sense to ship useShallow in a separate file?

[dbritto-dev]

@birtles you're right for now let's downgrade for now

[birtles]

Thank you!

[dai-shi]

I kind of knew this could be an issue, but not with what exactly the cases.

because I'm using Preact

and, okay it's the one.

In Jotai, we explicitly divide entry points. If we follow that it will be:

• zustand/shallow/vanilla for shallow
• zustand/shallow/react for useShallow
• zustand/shallow for both

I thought it's too much for Zustand.

I can easily alias 'react' to preact/compat

and, I hoped that workaround worked. In the end, if dead code elimination works well, it should be gone.

I wonder if there's a simpler alternative...

[birtles]

Right. The real problem is vitest's aliasing is too shallow. In webpack there is no problem since aliases can apply globally.

That said, splitting entry points that require react vs those that don't makes sense to me.

[dai-shi]

See vitest-dev/vitest#1652 (comment)

I wish vitest can do it. Does jest work or not?

cc: @Aslemammad

[birtles]

I haven't tried jest (it has poor ESM support so it's difficult to port back to jest) but I'm pretty sure it does as there's a preset for setting up the aliases: https://github.com/preactjs/jest-preset-preact

[Aslemammad]

Would something like this help you? https://pnpm.io/aliases

[birtles]

Yes, I think it would assuming it works with vitest. I would be nice to not have to switch package manager to work around this.

In other situations I've used node's experimental module policies to work around this. It does the trick but it's still experimental and I haven't tried using it with vite yet.

[dai-shi]

#2096 from @bkimminich.

Since 4.4.2 I am getting unexpected build failures in an application that has zustand only as a subdependency:

So, it's another case, hmm. See: #2095 (comment)

As I am not using zustand directly,

Even if you are not using it, is it correct that if you install react, it solves it right? Not ideal, but does it work at least?

[bkimminich]

Haven't tried adding react dependency, and that feels kind of worse than adding a pinned zustand dependency in an Angular app.

In general, I can't recommend it enough to stick to semantic versioning (https://semver.org) because then the fault would be at the dependents side if they specify their version range too wide. Everyone with ^4 or ~4.4.0 etc. would be just fine if this had been a 5.0.0 release. Just give it a try, I'm sure you won't want to go back to any other versioning approach... 😎

[dai-shi]

Are you using zustand in an angular app? Or, is one of your libs use zustand?

[bkimminich]

Yep, it's pulled in by the lib @wagmi/core:

frontend@15.2.0 C:\Data\GitHub\juice-shop\frontend
+-- @wagmi/core@0.5.8
| `-- zustand@4.4.1 deduped
`-- zustand@4.4.1

The additional direct dependency is only b/c I needed to pin 4.4.1 as @wagmi/core uses "zustand": "^4.0.0".

[dai-shi]

Thanks.
https://github.com/wagmi-dev/wagmi/blob/main/packages/core/package.json
Okay, there's no blame to anyone, but peer dependencies are hard to deal with.
For example, npm installs peer dependencies, so it's not a big issue.
But, it's still marked as an optional dependency, I think this is an issue.
It's not easy to work it around, we will seek for a solution.
Thanks for your reporting.

[dbritto-dev]

@dai-shi I'm wondering if would be better if we split zustand in two main packages like other FOSS projects, for instance redux (react package: react-redux) and @tanstack/query (react package: @tanstack/react-query)

They have two packages the non-react/core package and the react package.

They follow this convention: core-package-name and react-core-package-name in our case would be zustand and react-zustand

Where react-zustand has some peer does like zustand, react and use-sync-external-selector, etc.

And, zustand is just the vanilla version

We can import from react-zustand the next ones:

• create
• useShallow
• createWithEqualityFn
• useStore
• etc

We can import from zustand the next ones:

• createStore
• shallow
• etc

The goal with splitting the packages in two is not only separate the functions but also the code so people don't need to install extra packages when wants to work with the vanilla/non-react version

Note: the name of the packages could be zustand-vanilla and zustand

[dai-shi]

zustand/react-shallow is possible, but zustand/shallow/react is more consistent with jotai, and we can do zustand/shallow/vanilla and zustand/shallow in v5.
In this case, you don't need to install react, like it doesn't always require immer.

[dbritto-dev]

yeah, in the sense of being more consistent with jotai I guess zustand/shallow/react is OK but I would change it to <package>/react-shallow since zustand/shallow/react or zustand/shallow/vanilla feels a little bit hacky I mean feels more natural when you are one folder deep rather than two or more folders deep

[dbritto-dev]

@dai-shi btw, we need split our tests in two, the ones that just test vanilla and the other one that test react so if for some reason we add non require dep in vanilla the tests could catch it

[dai-shi]

Yeah, but we already have two-level zustand/middleware/immer.
Let's go with zustand/shallow/react for v4 for backward compatibility,
and in v5 we consider zustand/shallow/vanilla and zustand/shallow to have both.

It may feel differently from other FOSS's approach, but this is how we go in pmndrs org.

Are you willing to open a PR?

[dbritto-dev]

@dai-shi sure, I'll take care of that I'll do it as soon as possible