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(types): remove Partial typo from persist #1051

Merged
merged 4 commits into from
Jul 16, 2022

Conversation

devanshj
Copy link
Contributor

@devanshj devanshj commented Jul 3, 2022

In #725 I fixed persist types by making the default persisted state same as the whole state (which reflects the default behavior), but looks like I forgot to remove Partial from one place.

This is breaking change, it probably won't break for many people but a quickfix is to pass partialize: s => s as Partial<typeof s> in the options.

Remember it's a "quickfix" that is "I don't want more strict types, give me the old types". So the correct fix might be something else depending on the code. And again "might" implies the quickfix could be correct fix, eg the fix I apply for a test in this PR is not a quickfix but a correct fix as the partialized state is truly Partial<{ count: number }>.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 3, 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 8f66491:

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

@devanshj
Copy link
Contributor Author

devanshj commented Jul 3, 2022

Not sure why prettier in the CI has a non-zero exit.

@dai-shi
Copy link
Member

dai-shi commented Jul 5, 2022

It says:

$ prettier '*.{js,json,md}' '{src,tests,docs}/**/*.{ts,tsx,md,mdx}' --list-different
tests/persistSync.test.tsx
error Command failed with exit code 1.

Can you run yarn run prettier?

@devanshj
Copy link
Contributor Author

devanshj commented Jul 8, 2022

Ah so it's expected to exit non-zero because it's not formatted. I didn't catch that. I think it'd be nicer if the CI shows what exactly is ill-formatted (previously it used to do that).

(Also yarn run prettier didn't do anything, I had to yarn run prettier . --write and discard extra changes, maybe there's a better way)

@devanshj devanshj changed the title fix(types): remove Parital typo from persist fix(types): remove Partial typo from persist Jul 8, 2022
@dai-shi
Copy link
Member

dai-shi commented Jul 8, 2022

I hope it to run this script.

"prettier": "prettier '*.{js,json,md}' '{src,tests,docs}/**/*.{ts,tsx,md,mdx}' --write",

Maybe it works differently with different yarn versions?

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.

LGTM

@devanshj
Copy link
Contributor Author

devanshj commented Jul 8, 2022

Ah but the PR's package.json doesn't have that prettier script probably that's why it didn't work.

@dai-shi
Copy link
Member

dai-shi commented Jul 8, 2022

Oh, then, please merge main into this PR branch to make sure if everything is okay.

@dai-shi dai-shi added this to the v4.0.0 milestone Jul 11, 2022
@dai-shi dai-shi merged commit 81d7c4f into pmndrs:main Jul 16, 2022
devanshj added a commit to devanshj/zustand that referenced this pull request Jul 18, 2022
dai-shi pushed a commit that referenced this pull request Jul 18, 2022
* docs: Add migration for #1051

* better wording

* fix grammar, better wording
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.

2 participants