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

fix(traditional): make defaultEqualityFn optional in TS Types #2060

Merged
merged 1 commit into from
Oct 2, 2023

Conversation

charkour
Copy link
Collaborator

Summary

With the updates in v4.4.0 and the addition of the createWithEqualityFn, the types currently require defaultEqualityFn when the implementation itself is optional.

Thank you

Check List

  • yarn run prettier for formatting code and docs

@vercel
Copy link

vercel bot commented Sep 18, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
zustand-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 18, 2023 8:06pm

@codesandbox-ci
Copy link

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 40065b0:

Sandbox Source
React Configuration
React TypeScript Configuration
React Browserify Configuration
React Snowpack Configuration
React Parcel Configuration
Next.js Configuration
@pavlobu/zustand demo Configuration

@charkour
Copy link
Collaborator Author

Size Change: 0 B

Total Size: 43.5 kB

ℹ️ View Unchanged
Filename Size
dist/context.js 782 B
dist/esm/context.js 590 B
dist/esm/index.js 706 B
dist/esm/middleware.js 3.87 kB
dist/esm/middleware/immer.js 210 B
dist/esm/shallow.js 509 B
dist/esm/traditional.js 392 B
dist/esm/vanilla.js 629 B
dist/index.js 868 B
dist/middleware.js 4.43 kB
dist/middleware/immer.js 328 B
dist/shallow.js 1.11 kB
dist/system/context.development.js 697 B
dist/system/context.production.js 386 B
dist/system/index.development.js 873 B
dist/system/index.production.js 402 B
dist/system/middleware.development.js 4 kB
dist/system/middleware.production.js 2.62 kB
dist/system/middleware/immer.development.js 292 B
dist/system/middleware/immer.production.js 187 B
dist/system/shallow.development.js 558 B
dist/system/shallow.production.js 344 B
dist/system/traditional.development.js 505 B
dist/system/traditional.production.js 342 B
dist/system/vanilla.development.js 672 B
dist/system/vanilla.production.js 310 B
dist/traditional.js 433 B
dist/umd/context.development.js 887 B
dist/umd/context.production.js 556 B
dist/umd/index.development.js 999 B
dist/umd/index.production.js 543 B
dist/umd/middleware.development.js 4.58 kB
dist/umd/middleware.production.js 2.97 kB
dist/umd/middleware/immer.development.js 482 B
dist/umd/middleware/immer.production.js 342 B
dist/umd/shallow.development.js 1.23 kB
dist/umd/shallow.production.js 883 B
dist/umd/traditional.development.js 613 B
dist/umd/traditional.production.js 447 B
dist/umd/vanilla.development.js 800 B
dist/umd/vanilla.production.js 410 B
dist/vanilla.js 683 B

compressed-size-action

@dai-shi
Copy link
Member

dai-shi commented Sep 19, 2023

That was intentional to let people use it more explicitly, but I can accept this PR if people want. Please let us know if anyone else has opinion on this.

@charkour
Copy link
Collaborator Author

charkour commented Sep 21, 2023

Thanks for the quick response. If the equalityFn should be required, that is fine. There are two concerns that caused me to suggest changes:

  1. The v4.4.0 release notes are potentially misleading. Currently, it says // Object.is the default equalityFn which can be changed to shallow or anything else which suggests the equalityFn will be set to Object.is if no function is passed. If an equalityFn should be passed, this wording should be updated to say // Object.is is the default quailtyFn in zustand create; to match that behavior, pass that function here. This can be updated to shallow or anything else.
  2. The TypeScript function type signature does not match the actual function signature. On line 94 in traditional.ts, the equalityFn is optional, but on line 68 and 72, the parameter is required.

Whatever behavior you'd like is fine. I understand both ways. My goal is to avoid confusion by updating the docs or types. What do you think, @dai-shi?

@dai-shi
Copy link
Member

dai-shi commented Sep 22, 2023

1 is the right concern. We can update it.
2 is fine because we just limit types from the implementation. But it may confuse people.

Any thoughts from @dbritto-dev @sewera @chrisk-7777 ?

@sewera
Copy link
Collaborator

sewera commented Sep 22, 2023

I definitely agree with 1. 2 — I've got no opinion.

@dai-shi
Copy link
Member

dai-shi commented Sep 25, 2023

Updated v4.4.0 release note as suggested.

Basically, I would like to merge this PR. Just hope to get some more comments.

@dai-shi dai-shi changed the title bug(traditional): make defaultEqualityFn optional in TS Types fix(traditional): make defaultEqualityFn optional in TS Types Sep 25, 2023
@charkour
Copy link
Collaborator Author

The updated release notes read more clearly. Thanks, @dai-shi!

Copy link
Member

@dai-shi dai-shi left a comment

Choose a reason for hiding this comment

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

Let's merge this.

@dai-shi dai-shi added this to the v4.4.2 milestone Oct 2, 2023
@dai-shi dai-shi merged commit 368478b into pmndrs:main Oct 2, 2023
33 checks passed
bodinsamuel pushed a commit to specfy/specfy that referenced this pull request Nov 8, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [zustand](https://github.com/pmndrs/zustand) | [`4.4.1` ->
`4.4.6`](https://renovatebot.com/diffs/npm/zustand/4.4.1/4.4.6) |
[![age](https://developer.mend.io/api/mc/badges/age/npm/zustand/4.4.6?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/zustand/4.4.6?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/zustand/4.4.1/4.4.6?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/zustand/4.4.1/4.4.6?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>pmndrs/zustand (zustand)</summary>

### [`v4.4.6`](https://github.com/pmndrs/zustand/releases/tag/v4.4.6)

[Compare
Source](https://github.com/pmndrs/zustand/compare/v4.4.5...v4.4.6)

v4.4.5 has an issue with some TypeScript configs about module
resolution. It should be fixed now. Thanks for the patience.

#### What's Changed

- Update export types by
[@&#8203;dbritto-dev](https://github.com/dbritto-dev) in
[pmndrs/zustand#2170

#### New Contributors

- [@&#8203;cheatkey](https://github.com/cheatkey) made their first
contribution in
[pmndrs/zustand#2147
- [@&#8203;frixaco](https://github.com/frixaco) made their first
contribution in
[pmndrs/zustand#2166

**Full Changelog**:
pmndrs/zustand@v4.4.5...v4.4.6

### [`v4.4.5`](https://github.com/pmndrs/zustand/releases/tag/v4.4.5)

[Compare
Source](https://github.com/pmndrs/zustand/compare/v4.4.4...v4.4.5)

Hopefully, it should fix some issues with Node.js environment including
Next.js.

#### What's Changed

- fix: importing CJS React in ESM by
[@&#8203;dai-shi](https://github.com/dai-shi) in
[pmndrs/zustand#2154
- Apply publint recommendations by
[@&#8203;dbritto-dev](https://github.com/dbritto-dev) in
[pmndrs/zustand#2157

#### New Contributors

- [@&#8203;plrs9816](https://github.com/plrs9816) made their first
contribution in
[pmndrs/zustand#2137
- [@&#8203;Brammm](https://github.com/Brammm) made their first
contribution in
[pmndrs/zustand#2139
- [@&#8203;sobies93](https://github.com/sobies93) made their first
contribution in
[pmndrs/zustand#2142

**Full Changelog**:
pmndrs/zustand@v4.4.4...v4.4.5

### [`v4.4.4`](https://github.com/pmndrs/zustand/releases/tag/v4.4.4)

[Compare
Source](https://github.com/pmndrs/zustand/compare/v4.4.3...v4.4.4)

There was a tiny issue in v4.4.3, which broke with some bundlers, which
this version fixes.

#### What's Changed

- fix(build): patch entry points zustand/shallow for CJS by
[@&#8203;dai-shi](https://github.com/dai-shi) in
[pmndrs/zustand#2128

**Full Changelog**:
pmndrs/zustand@v4.4.3...v4.4.4

### [`v4.4.3`](https://github.com/pmndrs/zustand/releases/tag/v4.4.3)

[Compare
Source](https://github.com/pmndrs/zustand/compare/v4.4.2...v4.4.3)

The changes in v4.4.2 were troublesome for some users. This version
should fix/mitigate such cases.

#### What's Changed

- fix(shallow): Extract shallow vanilla and react by
[@&#8203;dbritto-dev](https://github.com/dbritto-dev) in
[pmndrs/zustand#2097
- fix(types): mitigate devtools typing by
[@&#8203;dai-shi](https://github.com/dai-shi) in
[pmndrs/zustand#2099

**Full Changelog**:
pmndrs/zustand@v4.4.2...v4.4.3

### [`v4.4.2`](https://github.com/pmndrs/zustand/releases/tag/v4.4.2)

[Compare
Source](https://github.com/pmndrs/zustand/compare/v4.4.1...v4.4.2)

This adds `useShallow` hook to cover some use cases that are deprecated
with v4.4.0 change. Check out [the
guide](https://github.com/pmndrs/zustand/blob/ec538e9d4c0b9b5759e6dfd0fd3c9a21f8236949/docs/guides/prevent-rerenders-with-use-shallow.md).

##### Migration Guide

[#&#8203;1991](https://github.com/pmndrs/zustand/issues/1991) requires
something like below if you are using the `devtools` middleware *and*
TypeScript.

```diff
  import { devtools } from 'zustand/middleware'
+ import type {} from '@&#8203;redux-devtools/extension'
```

##### What's Changed

- fix(types)(middleware/devtools): avoid copying types by
[@&#8203;dai-shi](https://github.com/dai-shi) in
[pmndrs/zustand#1991
- fix(traditional): make defaultEqualityFn optional in TS Types by
[@&#8203;charkour](https://github.com/charkour) in
[pmndrs/zustand#2060
- feat: add useShallow by
[@&#8203;FaberVitale](https://github.com/FaberVitale) in
[pmndrs/zustand#2090

##### New Contributors

- [@&#8203;aykutkardas](https://github.com/aykutkardas) made their
first contribution in
[pmndrs/zustand#1993
- [@&#8203;michelts](https://github.com/michelts) made their first
contribution in
[pmndrs/zustand#1997
- [@&#8203;elusive](https://github.com/elusive) made their first
contribution in
[pmndrs/zustand#2001
- [@&#8203;mayank1513](https://github.com/mayank1513) made their first
contribution in
[pmndrs/zustand#2015
- [@&#8203;fdb](https://github.com/fdb) made their first contribution
in
[pmndrs/zustand#2029
- [@&#8203;tmkx](https://github.com/tmkx) made their first
contribution in
[pmndrs/zustand#2032
- [@&#8203;OshriAsulin](https://github.com/OshriAsulin) made their
first contribution in
[pmndrs/zustand#2028
- [@&#8203;ivanquirino](https://github.com/ivanquirino) made their
first contribution in
[pmndrs/zustand#2047
- [@&#8203;stavkamil](https://github.com/stavkamil) made their first
contribution in
[pmndrs/zustand#2071
- [@&#8203;NaamuKim](https://github.com/NaamuKim) made their first
contribution in
[pmndrs/zustand#2088
- [@&#8203;FaberVitale](https://github.com/FaberVitale) made their
first contribution in
[pmndrs/zustand#2090

**Full Changelog**:
pmndrs/zustand@v4.4.1...v4.4.2

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 4pm every weekday" in timezone
Europe/Paris, Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/specfy/specfy).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40Ni4wIiwidXBkYXRlZEluVmVyIjoiMzcuNDYuMCIsInRhcmdldEJyYW5jaCI6ImNob3JlL3Jlbm92YXRlQmFzZUJyYW5jaCJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@charkour charkour deleted the ts-defaultEqualityFn branch April 6, 2024 16:50
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