Skip to content

Conversation

evenstensberg
Copy link

Swapped around so we don't need to use componentName = componentName || something

Does basically the same thing ( I think?? ) but does a conditional instead so we can cross out invoking the object iteself to check if it's an object or string.

Feedback would be great. Passed all tests, but please, do check if returning the string is proper!

❤️

@isaac-peka
Copy link

The former is much more expressive than the latter, and the premise of this being optimised is a false one. Why test for null and undefined explicitly when null and undefined are falsey?

function checkPropTypes(componentName, propTypes, props) {
componentName = componentName || 'UnknownComponent'

componentName = {} || ''
Copy link
Member

@ryanflorence ryanflorence Apr 15, 2016

Choose a reason for hiding this comment

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

this will always be {}, obliterating the arg coming in. What's the goal with this PR?

@evenstensberg
Copy link
Author

Thanks for the notification, swapping it around as we speak 😄

evenstensberg added 2 commits April 15, 2016 21:04
# Conflicts:
#	modules/RouteUtils.js
@evenstensberg
Copy link
Author

Closing this, fixing and let's resume this in the new PR, will go in detail there.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants