Skip to content

possible syntax clarification #600

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

Merged
merged 5 commits into from
Jan 16, 2017
Merged

Conversation

abglassford
Copy link
Contributor

I attempted to clarify some pronouns in a couple sentences to avoid confusion when sentence has multiple objects. Please let me know if I'm confusing some things.

I attempted to clarify some pronouns in a couple sentences to avoid confusion when sentence has multiple objects. Please let me know if I'm confusing some things.
@markerikson
Copy link
Contributor

Hmm. Appreciate the PR, but yeah, I think the new phrasing is incorrect. The idea is that when the store updates, the wrapper component (which is a subscriber to the store) will be notified. So, it's not that the component updates, it's the store.

To be honest, I think both paragraphs could stand to be rewritten from scratch. I'll have to add that to my todo list.

If you can fix up your edits to clarify that it's the store updates that lead to mapState being re-run, I'll get this in.

Rephrased wording to specify that the `mapStateToProps` will be called via the store being updated.
@abglassford
Copy link
Contributor Author

That seemed to be an easy fix. Unless I missed your point. Let me know if it's decent now.

@markerikson
Copy link
Contributor

Yeah, actually, let's make a couple more edits:

  • Change "If a component is specified" to "if this argument is specified"
  • Change "if you omit the component" to something more like "to skip subscribing to store updates, pass null or undefined for this argument (this applies for both mapState and mapDispatch)

@abglassford
Copy link
Contributor Author

abglassford commented Jan 16, 2017

So, when the argument mapStateToProps is specified, does it subscribe a component to store updates or it will subscribe itself to store updates? I just want to be clear on what is actually subscribing to the store.

edit

I think the next sentence answered my question:
"Any time the store is updated, mapStateToProps will be called."

see diff
@markerikson
Copy link
Contributor

markerikson commented Jan 16, 2017

connect generates a wrapper component. If mapState is supplied, that wrapper component will subscribe to the store, and when the store runs the subscriber callbacks, the wrapper component will call mapState(store.getState()).

Dan wrote a miniature version of connect that illustrates the basic idea: https://gist.github.com/gaearon/1d19088790e70ac32ea636c025ba424e

@abglassford
Copy link
Contributor Author

Gotcha. I'm going to leave it at this commit if it's sufficient. Thanks for taking the time to look at this and explain it to me!

@markerikson
Copy link
Contributor

One more phrasing tweak, if you could. Let's change "it will subscribe" to "the new component will subscribe".

Changed "it will subscribe" to "the new component will subscribe".
@markerikson markerikson merged commit a5aea01 into reduxjs:master Jan 16, 2017
@markerikson
Copy link
Contributor

Awright, looks good. Thanks for the help! If you've got any other suggestions or improvements for either the Redux or React-Redux docs, please feel free to file PRs and we'll see what we can do.

@abglassford
Copy link
Contributor Author

Thank you!

albertodev7 pushed a commit to albertodev7/react-redux that referenced this pull request Dec 8, 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