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

Add support for React 16 #190

Closed
wants to merge 1 commit into from
Closed

Add support for React 16 #190

wants to merge 1 commit into from

Conversation

joshwcomeau
Copy link
Owner

With React's official launch, I figured it was time to update the peer dependencies!

While trying to publish, I ran into some trouble; it wasn't able to find babel-cli when running the lib build step :/

Upgrading the dependency seems to fix it (publish still failed, but only because I hadn't yet incremented the version). I figured before I published, though, I should get another pair of eyes.

Mainly I'm curious if there's a more-ideal way to add support for React 16 in the peer dependencies.

@@ -1,6 +1,6 @@
{
"name": "react-flip-move",
"version": "2.9.14",
Copy link
Owner Author

Choose a reason for hiding this comment

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

Guess I never pushed the 2.9.15 branch, my bad. That was the release for zombie-nodes bugfix.

@Hypnosphi
Copy link
Collaborator

Did you test it with components returning strings or fragments?

@Hypnosphi
Copy link
Collaborator

a more-ideal way to add support for React 16 in the peer dependencies

How about wildcard?

Copy link
Collaborator

@tobilen tobilen left a comment

Choose a reason for hiding this comment

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

i'll spend most of tomorrow upgrading our codebase to react16. i'd like to give feedback after that.
my gut feeling though, some simple fail-safes should be implemented to raise/throw when fragments are being passed to flip-move.

something along the lines of ```js
if (typeof this.props.children === 'string' || typeof this.props.children === 'array') console.warn('react-flip-move doesnt support arrays or strings as components yet.');

@@ -46,14 +46,14 @@
"web-animations"
],
"peerDependencies": {
"react": "0.13.x || 0.14.x || 15.x.x",
"react-dom": "0.13.x || 0.14.x || 15.x.x"
"react": "0.13.x || 0.14.x || 15.x.x || 16.x.x",
Copy link
Collaborator

Choose a reason for hiding this comment

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

would "react": ">=0.13.x <=16.x.x", work?

@Hypnosphi
Copy link
Collaborator

Hypnosphi commented Sep 28, 2017

if (typeof this.props.children === 'string' || typeof this.props.children === 'array')

It should be not about this.props.children, but rather about output of each child's render.

@tobilen
Copy link
Collaborator

tobilen commented Oct 1, 2017

check is not needed anymore when we merge #191

@joshwcomeau
Copy link
Owner Author

This is no longer needed thanks to @tobilen's fantastic work :D

@joshwcomeau joshwcomeau closed this Oct 4, 2017
@joshwcomeau joshwcomeau deleted the react16 branch January 11, 2018 14:18
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.

None yet

3 participants