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 typo in typescript.md #1152

Merged
merged 3 commits into from
Aug 1, 2022
Merged

fix typo in typescript.md #1152

merged 3 commits into from
Aug 1, 2022

Conversation

garrett-hopper
Copy link
Contributor

Not sure if useBoundStore had some special meaning in other places, but there were incorrect references to it in the combine comment that didn't match the example. (I just replaced it with useBearStore everywhere assuming it was a typo.)

Not sure if `useBoundStore` had some special meaning in other places, but there were incorrect references to it in the `combine` comment that didn't match the example. (I just replaced it with `useBearStore` everywhere assuming it was a typo.)
@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 28, 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 73cb372:

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

@dai-shi
Copy link
Member

dai-shi commented Jul 28, 2022

useBoundStore was intentional but I knew it's confusing. I'm fine with useBearStore, but do you mean the animal bear 🐻 , right?

@garrett-hopper
Copy link
Contributor Author

Ah, ok, it may just need to be changed to useBoundStore here: https://github.com/pmndrs/zustand/pull/1152/files#diff-ae03a9f2eb638a25c1d8d7ba853c78220f1ed44a19ad97a8d780cbc0c00a4260R124
The "But be a little careful" comment below it references useBoundStore rather than useBearStore as is in the combine example. (I'm guessing to match the BearState 🐻)
You could keep useBoundStore for the first and last change I'd made or even just change the useBearStore example to be useBoundStore matching its comment. 🤷
Feel free to close this and update it as you see fit.

@garrett-hopper
Copy link
Contributor Author

@dai-shi
Copy link
Member

dai-shi commented Jul 29, 2022

it may just need to be changed to useBoundStore here

Can you update the PR with your new suggestion?

@garrett-hopper
Copy link
Contributor Author

Done; now only the references that are actually incorrect have been updated. (Still feels weird to have both useBoundStore and useBearStore names as examples, but 🤷)

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
The naming of useBoundStore is pretty new in #1125. I'm not confident either. There might be a better idea, or just an education problem.

@dai-shi dai-shi merged commit 0d6ff79 into pmndrs:main Aug 1, 2022
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