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 broken example in the README.md #92

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

boazberman
Copy link

In the example store.setState was called with what could be a stale state. In order to use the current state, one must call store.getState. Similar to how in React if you call setState without a callback it is a potential bug.

In the example store.setState was called with what could be a stale state. In order to use the current state, one must call store.getState. Similar to how in React if you call setState without a callback it is a potential bug.
@boazberman
Copy link
Author

boazberman commented Jun 16, 2018

I've spent a lot of time searching for this API, and only found it in the declaration file.
If I'm mistaking for it's need, I would be more then grateful to learn.

@developit
Copy link
Owner

I believe there's another example that shows the getState() version - perhaps we'd be better off showing a synchronous function for this example? I just wanted to have a way to show both.

@boazberman
Copy link
Author

Well, my difficulty was that I just could not understand how one can manipulate the state object after an async operation happened, and expect the results to be valid, as the state could change during that period, but the state object being manipulated is the old state. If you think there is no need in this example I understand, I just though it will save some people from having bugs caused by an asynchronous behavior. Thanks!

@boazberman boazberman closed this Aug 31, 2018
@boazberman boazberman reopened this Aug 31, 2018
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