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

Docs: clarify action merging behavior when combining slices #30

Merged
merged 1 commit into from
Sep 27, 2024

Conversation

unbeauvoyage
Copy link
Contributor

@unbeauvoyage unbeauvoyage commented Sep 27, 2024

When I tried the code myself, I wasn't expecting the actions to be merged and thought it was a mistake. It was concluded, in another discussion that 'this "action merging" behavior is one of features of this fairly opinionated lib, so it should be clearly stated somewhere'. To prevent similar confusion when others try it too, I thought that it would be best not to let it be forgotten and that "combining slices" part in the getting started section, right after the relevant example, is a good place for a clarification. Feel free to edit the change or altogether reject it if it is not the proper place.


Below is how it will look in the docs:

image

When I tried the code myself, I wasn't expecting the actions to be merged and thought it was a mistake. It was concluded, in another discusstion, similar confusion might arise when others try it too.
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.

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.

Thanks for adding the note. Looks good to me!

Just to let you know, I understand this behavior can be confusing. If it's not welcomed by many users, this library may reconsider its opinion in the future.

@dai-shi dai-shi merged commit 07b8fe7 into zustandjs:main Sep 27, 2024
2 checks passed
@unbeauvoyage
Copy link
Contributor Author

@dai-shi
In real world applications slices will usually be in different files and there could be dozens of properties (even hundreds, if the app is using a single store), and it might cause name collisions and bugs. There will be higher likelihood of developers being caught off guard and an additional layer they need to be aware of. Since this library already provides withActions() for store level actions, where one can easily compose individual actions anyway, I would humbly suggest that it might be worth reconsidering this approach.

@dai-shi
Copy link
Member

dai-shi commented Sep 27, 2024

Thanks for the suggestion. Can you open a new issue to discuss further?

I don't dislike the suggested approach. It's straightforward and the implementation can be simpler.

dai-shi added a commit that referenced this pull request Sep 29, 2024
dai-shi added a commit that referenced this pull request Oct 11, 2024
* feat: avoid action merging

* revert #30

* support bail-out

* update CHANGELOG
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