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

[0.45] react@16 did away with PropTypes; require prop-types instead #14640

Closed
wants to merge 1 commit into from

Conversation

mojodna
Copy link
Contributor

@mojodna mojodna commented Jun 20, 2017

react@16 (a peerDependency) did away with the PropTypes export in favor of the prop-types module.

This updates all of the remaining references to ReactPropTypes (and yes, targets the 0.45-stable branch; I'll submit a PR that targets master as well).

@facebook-github-bot
Copy link
Contributor

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. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot facebook-github-bot 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 Jun 20, 2017
@facebook-github-bot
Copy link
Contributor

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

@hramos
Copy link
Contributor

hramos commented Jun 22, 2017

Let's wait for #14641 to land first, then we can cherry-pick the commit from master.

@hramos hramos changed the title react@16 did away with PropTypes; require prop-types instead [0.45] react@16 did away with PropTypes; require prop-types instead Jun 22, 2017
@mojodna
Copy link
Contributor Author

mojodna commented Jun 22, 2017

FWIW, this commit includes other fixes that got taken care of somewhere on the way to the current master, so cherry picking won't hit all of the spots.

@Jonovono
Copy link

What's the status on this? Currently React Native 0.45.1 breaks with React 16.0.0 a 12 because of the prop types.

@hramos
Copy link
Contributor

hramos commented Jun 23, 2017

@mojodna which commits need to be cherry picked? If I can get a list of those commits already on master, that we can cherry pick onto 0.45 and thus cut a 0.45.2 release, I can get that moving.

@hramos
Copy link
Contributor

hramos commented Jun 23, 2017

We won't be doing this via PR, so let's move the discussion to the following issue: #14712

@hramos hramos closed this Jun 23, 2017
@mojodna mojodna deleted the v0.45.1+prop-types branch June 26, 2017 18:20
facebook-github-bot pushed a commit that referenced this pull request Jul 25, 2017
Summary:
The following PR modifies the Danger rules in the following way:

1. Verifies if a PR is opened against master. If not, it will warn (if opened against stable) or fail (anything else).
2. No longer adds a markdown message tagging the facebook/react-native team, as the bot does not have the necessary scope to mention the team.
3. Mentions people that have marked themselves as interested in a file, when that file is modified. This is based off CODEOWNERS. The bot should be able to use mentions here as it will act as any other regular user.

Verify it tags the right people in #15139

```
$ npm run danger pr #15139

> @ danger /Users/hramos/git/react-native/danger
> node ./node_modules/.bin/danger "pr" "#15139"

{
  fails: [],
  warnings: [],
  messages: [],
  markdowns: ["Attention: grabbou, kureev"]
}
```

It should not tag anyone for #15175:

```
$ npm run danger pr #15175

> @ danger /Users/hramos/git/react-native/danger
> node ./node_modules/.bin/danger "pr" "#15175"

{
  fails: [],
  warnings: [],
  messages: [],
  markdowns: []
}
```

It should warn on #14640 as it targets 0.45-stable:

```
$ npm run danger pr #14640

> @ danger /Users/hramos/git/react-native/danger
> node ./node_modules/.bin/danger "pr" "#14640"

{
  fails: [],
  warnings: [
    {
      message: ":grey_question: Base Branch - <i>The base branch for this PR is something other than `master`. Are you sure you want to merge these changes into a stable release? If you are interested in backporting updates to an older release, the suggested approach is to land those changes on `master` first and then cherry-pick the commits into the branch for that release. The [Releases Guide](https://github.com/facebook/react-native/blob/master/Releases.md) has more information.</i>"
    }
  ],
  messages: [],
  markdowns: [":page_facing_up: Thanks for your contribution to the docs!"]
}
```

It should not warn on #15175 because it targets master.
```
$ npm run danger pr #15175

> @ danger /Users/hramos/git/react-native/danger
> node ./node_modules/.bin/danger "pr" "#15175"

{
  fails: [],
  warnings: [],
  messages: [],
  markdowns: []
}
```
Closes #15179

Differential Revision: D5490047

Pulled By: hramos

fbshipit-source-id: a46a23b7d0a59d12b8039746d6e9c4399ef32d5f
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.

4 participants