-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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: add an example for setting the state outside event (#302) #303
Conversation
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 22ad473:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion. Would you please make the example code minimal? We'd like to make readme as small as possible. (We could create a wiki page and add a link.)
What do you think about recommending using batchedUpdates
in store creator, in addAFish
?
readme.md
Outdated
} | ||
``` | ||
|
||
More details: https://twitter.com/dan_abramov/status/959507572951797761 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how this is helpful. Remove it or replace the link to #302 and add the twitter link there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
@dai-shi I'm not sure what should I delete from the example to make it smaller. As I tried to make it as minimal as possible. Any suggestions? Regarding moving the batchFunction inside the action, I did not added there in order to better point out were the issue comes from. I can move it if you think it makes more sense. |
I understand your point. Let's keep it as is and we will see. |
Co-authored-by: Daishi Kato <dai-shi@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merging this.
Thanks for this addition @ratoi-crysty and @dai-shi, I think this is helpful for more people. |
close #302.