Skip to content

Remove getDefaultProps from plain Javascript class#9241

Merged
ide merged 1 commit into
react:0.31-stablefrom
romidane:patch-1
Aug 8, 2016
Merged

Remove getDefaultProps from plain Javascript class#9241
ide merged 1 commit into
react:0.31-stablefrom
romidane:patch-1

Conversation

@romidane

@romidane romidane commented Aug 5, 2016

Copy link
Copy Markdown

motivation
getDefaultProps is not available in ES6 class declaration for components. Adding it here may cause confusion to new comers.

@ghost

ghost commented Aug 5, 2016

Copy link
Copy Markdown

By analyzing the blame information on this pull request, we identified @hramos and @scarlac to be potential reviewers.

@ghost

ghost commented Aug 5, 2016

Copy link
Copy Markdown

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

@ghost ghost added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Aug 5, 2016
@ghost

ghost commented Aug 5, 2016

Copy link
Copy Markdown

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@ide

ide commented Aug 5, 2016

Copy link
Copy Markdown
Contributor

The only issue was the syntax, can you add back the default props?

class X {
  static defaultProps = {
    stuff
  };
}

@ghost ghost added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. GH Review: review-needed and removed GH Review: needs-revision labels Aug 5, 2016
@ghost

ghost commented Aug 5, 2016

Copy link
Copy Markdown

@romidane updated the pull request.

@romidane

romidane commented Aug 5, 2016

Copy link
Copy Markdown
Author

@ide : Sure thing I've added back the default props.

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 5, 2016
Comment thread docs/UsingNavigators.md Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

trailing commas

@ghost

ghost commented Aug 6, 2016

Copy link
Copy Markdown

@romidane updated the pull request.

@romidane

romidane commented Aug 6, 2016

Copy link
Copy Markdown
Author

@ide : I made the change. Had to double just with the style guide :)

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 6, 2016
Comment thread docs/UsingNavigators.md Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you revert this?

@ide

ide commented Aug 6, 2016

Copy link
Copy Markdown
Contributor

I noticed this PR is to the 0.31-stable branch. Can you please resubmit a PR to master?

and add PropType validation

* getDefaultProps is not available in ES6 class style declaration
@ghost

ghost commented Aug 7, 2016

Copy link
Copy Markdown

@romidane updated the pull request.

@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 7, 2016
@romidane

romidane commented Aug 7, 2016

Copy link
Copy Markdown
Author

@ide: Seems like the syntax problem has been resolved on master already. I think this PR would only be benficial as a patch to this tag?

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 7, 2016
@ide

ide commented Aug 8, 2016

Copy link
Copy Markdown
Contributor

Ah ok, will merge in that case.

@ide ide merged commit 9864c3a into react:0.31-stable Aug 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants