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: add an example for setting the state outside event (#302) #303

Merged
merged 6 commits into from
Feb 11, 2021
Merged

docs: add an example for setting the state outside event (#302) #303

merged 6 commits into from
Feb 11, 2021

Conversation

ratoi-crysty
Copy link
Contributor

@ratoi-crysty ratoi-crysty commented Feb 11, 2021

close #302.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 11, 2021

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:

Sandbox Source
React Configuration
React Typescript Configuration

@ratoi-crysty ratoi-crysty changed the title feat(docs): add an example for setting the state outside event (#302) docs: add an example for setting the state outside event (#302) Feb 11, 2021
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 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
Copy link
Member

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK

@ratoi-crysty
Copy link
Contributor Author

@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.

readme.md Outdated Show resolved Hide resolved
@dai-shi
Copy link
Member

dai-shi commented Feb 11, 2021

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>
readme.md Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
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.

Merging this.

@dai-shi dai-shi merged commit c31c710 into pmndrs:master Feb 11, 2021
@Tirzono
Copy link
Contributor

Tirzono commented Feb 11, 2021

Thanks for this addition @ratoi-crysty and @dai-shi, I think this is helpful for more people.

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.

Zombie child, yet again
3 participants