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

Warns on access of props.key and props.ref #5744

Merged

Conversation

ManasJayanth
Copy link
Contributor

Attempt for fix 5742

@ManasJayanth
Copy link
Contributor Author

Link to SO could be change to a note in the docs

@ManasJayanth ManasJayanth changed the title Warns on access of props.key and props.ref [WIP] Warns on access of props.key and props.ref Dec 29, 2015
'in `undefined` being returned. If you need to access the same ' +
'value within the child component, you should pass it as a different ' +
'prop. (http://stackoverflow.com/questions/33682774/how-to-access-the' +
'-key-property-from-a-reactjs-component)',
Copy link
Contributor

Choose a reason for hiding this comment

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

This link explains about accessing key and not ref. Might be confusing to the reader.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. The idea is to replace it with a link to relevant documentation being discussed at #5740

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR needs more work too. Its failing some tests

Copy link
Contributor

Choose a reason for hiding this comment

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

We generally use fb.me urls that point to gists. I created one for you to use: https://fb.me/react-special-props

'%s: `key` is not a prop. Trying to access it will result ' +
'in `undefined` being returned. If you need to access the same ' +
'value within the child component, you should pass it as a different ' +
'prop. (http://stackoverflow.com/questions/33682774/how-to-access-the' +
Copy link
Contributor

Choose a reason for hiding this comment

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

@facebook-github-bot
Copy link

@prometheansacrifice updated the pull request.

@ManasJayanth ManasJayanth changed the title [WIP] Warns on access of props.key and props.ref Warns on access of props.key and props.ref Jan 18, 2016
@ManasJayanth
Copy link
Contributor Author

@jimfb This PR is ready to be reviewed. Can you please have a look?

'prop. (https://fb.me/react-special-props)',
'displayName' in type ? type.displayName: 'Element'
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's explicitly add a return statement here. This is a getter, so we should return a value, and it makes the expected behavior a little more clear. Same with the getter for ref.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. I'll wait for @spicyj's comment on the next line note and update this PR afterwards.

@facebook-github-bot
Copy link

@prometheansacrifice updated the pull request.

@jimfb
Copy link
Contributor

jimfb commented Feb 16, 2016

@prometheansacrifice Can you add an explicit return statement as per above, and then I think this is ready to go.

@ManasJayanth
Copy link
Contributor Author

@jimfb Sure

@jimfb
Copy link
Contributor

jimfb commented Feb 16, 2016

Don't forget to add it to both getters (key+ref), even though I just called out one of them.

@ManasJayanth ManasJayanth force-pushed the warn-if-user-accesses-key-ref-props branch from 55607a8 to c3980a6 Compare February 16, 2016 16:37
@facebook-github-bot
Copy link

@prometheansacrifice updated the pull request.

@jimfb
Copy link
Contributor

jimfb commented Feb 16, 2016

This looks great, thanks @prometheansacrifice!

jimfb added a commit that referenced this pull request Feb 16, 2016
…es-key-ref-props

Warns on access of `props.key` and `props.ref`
@jimfb jimfb merged commit 3bee2d9 into facebook:master Feb 16, 2016
und3fined pushed a commit to und3fined/material-ui that referenced this pull request Apr 20, 2016
Calls to props.ref will throw a warning in React v15.  This commit hard-codes the
value instead of reading props.ref.  The iconButtonRef element has been removed
from the state object, since it never changes after the component is initialized.

React warnings discussed in this PR:
facebook/react#5744
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warn on access of props.key and props.ref
6 participants