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

Remove prop types checking in ReactCompositeComponent #6824

Merged
merged 2 commits into from
May 23, 2016

Conversation

keyz
Copy link
Contributor

@keyz keyz commented May 21, 2016

Implements #6791. I renamed _checkPropTypes() to _checkContextTypes() since it only checks context types for now.
Moreover, now ReactElementValidator's checkPropTypes() and ReactCompositeComponent's _checkContextTypes() are pretty much the same and we can merge them. Should I create a separate PR to refactor them or just change them here? @spicyj

@@ -830,7 +794,7 @@ var ReactCompositeComponentMixin = {
// warning for DOM component props in this upgrade
nextProps = nextParentElement.props;
} else {
nextProps = this._processProps(nextParentElement.props);
nextProps = nextParentElement.props;
willReceive = true;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you simplify this to

nextProps = nextParentElement.props;
if (prevParentElement !== nextParentElement) {
  willReceive = true;
}

now?

@sophiebits sophiebits added this to the 15.y.z milestone May 23, 2016
false,
'Failed Context Types: %s%s',
error.message,
addendum
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you mind switching this to use getStackAddendumByID to show the stack? That can be a separate PR if you prefer.

@sophiebits
Copy link
Collaborator

It's not obvious to me whether the two methods can be easily combined because they behave a little differently with the addendum (and it would be nice to calculate the addendum only if there is a warning) but if you see a way to clean them up let's do that in a follow-up PR.

@sophiebits
Copy link
Collaborator

After fixing the inline comments, feel free to merge.

@ghost
Copy link

ghost commented May 23, 2016

@keyanzhang updated the pull request.

@keyz keyz merged commit c136369 into facebook:master May 23, 2016
@keyz keyz deleted the hey-prop-types-just-once branch May 23, 2016 18:13
zpao pushed a commit to zpao/react that referenced this pull request Jun 8, 2016
Remove prop types checking in ReactCompositeComponent

(cherry picked from commit c136369)
zpao pushed a commit that referenced this pull request Jun 14, 2016
Remove prop types checking in ReactCompositeComponent

(cherry picked from commit c136369)
@zpao zpao modified the milestones: 15-next, 15.2.0 Jun 14, 2016
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.

3 participants