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

Upgrade to React 16 #387

Merged
merged 15 commits into from
Sep 29, 2017
Merged

Upgrade to React 16 #387

merged 15 commits into from
Sep 29, 2017

Conversation

larixer
Copy link
Member

@larixer larixer commented Sep 25, 2017

This is an early attempt to upgrade the kit to React 16. The full upgrade will be ready when all dependencies are ready for React 16, or when alternatives found for these dependencies.

Enzyme is not ready for React 16, so tests are disabled for now.

@larixer
Copy link
Member Author

larixer commented Sep 25, 2017

Web and Mobile part works for me, though more thorough checking is needed. The only thing that is not working is UI tests, due to Enzyme issues

@mitjade
Copy link
Collaborator

mitjade commented Sep 26, 2017

@Vlasenko https://github.com/airbnb/enzyme just got updated to v3.0.0, that should support react v16

@larixer
Copy link
Member Author

larixer commented Sep 26, 2017

@mitjade Many thanks for the hint Mitja! I will check now

@larixer
Copy link
Member Author

larixer commented Sep 26, 2017

@mitjade Okay, I have tried, it doesn't work for me with React 16. Well it kinda works but have lots of bugs, that makes it usage impractical

@larixer
Copy link
Member Author

larixer commented Sep 26, 2017

@mitjade So Enzyme v3 has problems with traversing React 16 tree. Except UI tests, everything else works fine in this branch.

@mitjade
Copy link
Collaborator

mitjade commented Sep 26, 2017

@Vlasenko Great! Hopefully it will get fixed by the time react is released and then you can merge this, or did you plan to do it sooner?

@larixer
Copy link
Member Author

larixer commented Sep 26, 2017

@mitjade I plan to do this right on the day when React 16 get finally released, if Enzyme will be usable on that date as well.

@mitjade
Copy link
Collaborator

mitjade commented Sep 26, 2017

@Vlasenko Sound good.

@mitjade
Copy link
Collaborator

mitjade commented Sep 26, 2017

@larixer
Copy link
Member Author

larixer commented Sep 26, 2017

@mitjade Yes, I saw React 16 release. But Enzyme@3 doesn't work for me neither for React 16 nor for React 15.4

@larixer
Copy link
Member Author

larixer commented Sep 29, 2017

Enzyme 3 is not working properly with React 16 at the moment:
enzymejs/enzyme#1163

@mairh @mitjade What should we do? Wait for the fix, or disable UI tests for now and merge this PR?

@mairh
Copy link
Member

mairh commented Sep 29, 2017

I think if we wait more this PR will go into the stale mode and probably you will end up fixing all other packages based on their future updates. So, I'll say merge this and disable the tests for now.

@mitjade
Copy link
Collaborator

mitjade commented Sep 29, 2017

@Vlasenko I agree with @mairh. Let's merge and deal with tests later on.

@mairh
Copy link
Member

mairh commented Sep 29, 2017

I am sure you have noticed the merge conflicts in this PR.

@larixer
Copy link
Member Author

larixer commented Sep 29, 2017

@mairh @mitjade I was able to find workarounds for Counter UI tests for Enzyme 3. So we have at least one working UI tests example for React 16.

@larixer larixer merged commit 046bd41 into master Sep 29, 2017
@mairh mairh deleted the react16 branch September 29, 2017 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants