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.2] Commits to cherry-pick and use prop-types with React 16 instead #14712

Closed
hramos opened this issue Jun 23, 2017 · 17 comments
Closed

[0.45.2] Commits to cherry-pick and use prop-types with React 16 instead #14712

hramos opened this issue Jun 23, 2017 · 17 comments
Labels
Stale There has been a lack of activity on this issue and it may be closed soon.

Comments

@hramos
Copy link
Contributor

hramos commented Jun 23, 2017

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

The issue has been fixed in master. @mojodna has indicated interest in getting this cherry-picked into the 0.45 release. Several issues have been opened that seem to be related to this, enough that I think it's worth cherry picking into 0.45.2.

@hramos
Copy link
Contributor Author

hramos commented Jun 23, 2017

@mojodna do you have a list of the commits in master that need to be cherry picked into 0.45 in order to fix the issue?

I've also opened #14713 as we'll also need to cherry pick them into the 0.46 RC in order to make sure the stable release doesn't suffer from this next month.

@mojodna
Copy link
Contributor

mojodna commented Jun 24, 2017

I don't, but I'll go hunting now.

@mojodna
Copy link
Contributor

mojodna commented Jun 24, 2017

@Ashoat
Copy link
Contributor

Ashoat commented Jun 25, 2017

One extant issue in master is that many components, such as Text, are still implemented with React.createClass (for the mixins functionality, I assume). Without this resolved React Native will error out on any attempt to use these components, so it should probably be considered blocking for 0.45.2.

It seems that 0.45.1 is pretty unusable in the meantime. Is there a recommended workaround, or should everybody downgrade to earlier versions? If we should downgrade, is it worth considering withdrawing the release, considering that it's unusable?

@Ashoat
Copy link
Contributor

Ashoat commented Jun 25, 2017

Curiously, there have been folks reporting (on #14588) that they've been experiencing this issue intermittently, and I've noticed that two weeks ago I was able to use react-native@0.45.0-rc.2/react@16.0.0-alpha.12 without issue and now am not. It would seem that react@16.0.0-alpha.12 should never have been usable with any version of React Native (including master which still has extant React.createClass references), but there are folks for whom this has worked. Perhaps it has something to do with how React Native or the packager resolves the 'react' package internally?

EDIT

Somehow I managed to get RN 0.45.1 to work by downgrading React to 16.0.0-alpha.6 and then updating back to 16.0.0-alpha.12. I have no idea what's going on. I'm using npm@5.0.3.

@hramos
Copy link
Contributor Author

hramos commented Jun 25, 2017

@Ashoat generally we recommend that people use the release candidate, in order to surface issues like these before a stable release is cut. Since the createClass issue is still present in master, we just need someone to send a PR to fix it. We're less than a week away from a stable release cut, so ideally this would be done within the next 24 hours so that we can get it into the RC early enough for people to test.

@Ashoat
Copy link
Contributor

Ashoat commented Jun 25, 2017

@hramos - believe it or not, I have been using the release candidates! But as mentioned before, these issues only randomly started popping up when I tried updating to RN 0.45.1. I'm really not sure what's going on, and why these errors are only triggered in certain situations.

Regarding the React.createClass issue, it's worth understanding just how many components in React Native are defined this way. This is just the portion in the Libraries/Components folder: https://gist.github.com/Ashoat/b68064b091d6389080acdec6ee1e7bd7

I doubt anybody's going to be able to put up a pull request fixing those within 24 hours, which means people will ostensibly continue seeing this error in 0.45.2. The big question in my mind is: why isn't everybody seeing these errors? Based on the claim in the error, it would seem that nobody should be able to use React Native 0.45 with React 16, but for some reason this issue does not occur in certain situations. For instance, my dev environment has been in a working state ever since I reinstalled react@16.0.0-alpha.12.

@mojodna
Copy link
Contributor

mojodna commented Jun 26, 2017

#14729 (658c7bb) should be cherry-picked into the 0.45 branch prior to 0.45.2 to address the React.createClass deprecation. (@hramos - this should probably be its own issue?)

@ozburo
Copy link

ozburo commented Jul 4, 2017

@Ashoat "The big question in my mind is: why isn't everybody seeing these errors?"

We are, we're just waiting patiently for fixes so we can upgrade w/o issues :D

I'm still on RN v0.44.3 b/c every attempt to upgrade results in an array of errors which seem to stem from a "catch-22" situation of not having the right version of React in sync with the RN and all the stuff that has been depreciated and/or changed (read: PropTypes etc.).

I for one don't know what to do, I'm still using React v16.0.0-alpha.6, tried updating that when I upgrade RN which is automagic since newer versions of RN require newer alpha versions of React_, but alas, no deal.

It would be great to figure out what are the correct update steps we need to keep our codebases up to date, I'm stuck cherrypicking files directly from master so I can at least get the lastest version of some Components, like SectionList, for example.

@grabbou
Copy link
Contributor

grabbou commented Jul 6, 2017

I'll be releasing new version later today / tomorrow morning with those fixes cherry-picked.

facebook-github-bot pushed a commit that referenced this issue Jul 7, 2017
Summary:
This replaces all uses of `React.createClass` with `createReactClass` from the `create-react-class` package, attempting to match use of `var` and `const` according to local style.

Fixes #14620
Refs #14712
Closes #14729

Differential Revision: D5321810

Pulled By: hramos

fbshipit-source-id: ae7b40640b2773fd89c3fb727ec87f688bebf585
grabbou pushed a commit that referenced this issue Jul 11, 2017
Summary:
This replaces all uses of `React.createClass` with `createReactClass` from the `create-react-class` package, attempting to match use of `var` and `const` according to local style.

Fixes #14620
Refs #14712
Closes #14729

Differential Revision: D5321810

Pulled By: hramos

fbshipit-source-id: ae7b40640b2773fd89c3fb727ec87f688bebf585
@peacechen
Copy link

peacechen commented Jul 12, 2017

Where is 0.45.2 available? I don't see it on npm at the moment.

Edit: What's the timeline for a 0.45.2 release? 0.46 isn't stable when I tried it out, and there are a number of critical issues reported against it.

@hramos
Copy link
Contributor Author

hramos commented Jul 12, 2017

I don't think it has been released yet. You can see there's no 0.45.2 tag on the repo.

hramos pushed a commit that referenced this issue Jul 12, 2017
Summary:
This replaces all uses of `React.createClass` with `createReactClass` from the `create-react-class` package, attempting to match use of `var` and `const` according to local style.

Fixes #14620
Refs #14712
Closes #14729

Differential Revision: D5321810

Pulled By: hramos

fbshipit-source-id: ae7b40640b2773fd89c3fb727ec87f688bebf585
@hramos
Copy link
Contributor Author

hramos commented Jul 12, 2017

Waiting on https://circleci.com/gh/facebook/react-native/19525

hramos pushed a commit that referenced this issue Jul 12, 2017
Summary:
This replaces all uses of `React.createClass` with `createReactClass` from the `create-react-class` package, attempting to match use of `var` and `const` according to local style.

Fixes #14620
Refs #14712
Closes #14729

Differential Revision: D5321810

Pulled By: hramos

fbshipit-source-id: ae7b40640b2773fd89c3fb727ec87f688bebf585
@iegik
Copy link

iegik commented Aug 24, 2017

e7c1cf5 resolves #14838

@grabbou
Copy link
Contributor

grabbou commented Sep 16, 2017

cc: @hramos build never made it to npm, I believe we should fix it. Do you think there's someone at Facebook that could assist me in making this on npm?

@hramos
Copy link
Contributor Author

hramos commented Sep 19, 2017

It did not publish to npm due to tests not being green. This will affect any release cut when tests aren't passing. We'll need to make a decision whether tests failing on Circle should block npm releases or not.

If you want to get this out anyway, I believe commenting out the failing tests out of circle.yml should do the trick.

@stale
Copy link

stale bot commented Nov 18, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. If you think this issue should definitely remain open, please let us know why. Thank you for your contributions.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Nov 18, 2017
@stale stale bot closed this as completed Nov 25, 2017
@facebook facebook locked and limited conversation to collaborators May 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Stale There has been a lack of activity on this issue and it may be closed soon.
Projects
None yet
Development

No branches or pull requests

7 participants