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: Fix Code Example for API connect() #1413

Merged
merged 1 commit into from
Oct 2, 2019
Merged

Docs: Fix Code Example for API connect() #1413

merged 1 commit into from
Oct 2, 2019

Conversation

duncanleung
Copy link
Contributor

The docs: Using React Redux > Connect: Extracting Data with mapStateToProps

Specify the following behavior in the code example:

function mapStateToProps(state) {
  console.log(state) // state
  console.log(arguments[1]) // undefined
}
const mapStateToProps = (state, ownProps = {}) => {
  console.log(state) // state
  console.log(ownProps) // undefined
}

These docs for connect() show a code example that is not consistent with above example, nor the explanation: API Reference > connect()

ownProps is not passed to mapStateToProps and mapDispatchToProps if the formal definition of the function contains one mandatory parameter (function has length 1). For example, functions defined like below won't receive ownProps as the second argument

@netlify
Copy link

netlify bot commented Oct 1, 2019

Deploy preview for react-redux-docs ready!

Built with commit 186c42a

https://deploy-preview-1413--react-redux-docs.netlify.com

@timdorr
Copy link
Member

timdorr commented Oct 2, 2019

Thanks!

@timdorr timdorr merged commit d9c49fa into reduxjs:master Oct 2, 2019
@markerikson
Copy link
Contributor

I'm pretty sure the prior docs were correct, and this example is wrong. Here's why.

First, let's see what happens when we define a function with a default argument, and check the .length:

const fn1 = (a, b={}) => {}
console.log(fn1.length) // 1

Since a mapState with a default argument for ownProps would have a length of 1, connect will indeed call it as mapState(state), not mapState(state, wrapperProps).

That means that the ownProps argument will be undefined, and therefore, it would fall back to the empty object default argument that the user actually specified.

Looking at the diff, the prior example did use to say console.log(ownProps) // {}. That would be correct, and so this needs to be reverted.

Appreciate the attempt to improve things, though!

@duncanleung
Copy link
Contributor Author

@markerikson @timdorr

Should I make the code example consistent in Using React Redux > Connect: Extracting Data with mapStateToProps > The Number of Declared Arguments Affects Behavior

The same code example is in those docs, but still shows:

function mapStateToProps(state) {
  console.log(state) // state
  console.log(arguments[1]) // undefined
}

const mapStateToProps = (state, ownProps = {}) => {
  console.log(state) // state
  console.log(ownProps) // undefined <- should be // {} ?
}

@markerikson
Copy link
Contributor

Yeah, maybe tweak that example to clarify it (the incoming value is undefined, which thus results in the default argument value being used).

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.

3 participants