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

Update for React 0.14.x #371

Closed
mxdubois opened this issue Aug 10, 2015 · 18 comments
Closed

Update for React 0.14.x #371

mxdubois opened this issue Aug 10, 2015 · 18 comments

Comments

@mxdubois
Copy link

Hey,

I know it's a bit early, as React 0.14.x is still in beta, but sometime soon it'd be good to get a branch going with 0.14.x support.

I'm seeing these warnings:

Warning: `require("react").findDOMNode` is deprecated. Please use `require("react-dom").findDOMNode` instead.

Everything else seems to be fine, though.

@bruderstein
Copy link
Collaborator

Could this be done with a one-time check? Something like

var React = require('react');

var findDOMNode = React.findDOMNode;
if (React.version.split('.')[1] >= '14') {
   findDOMNode = require('react-dom').findDOMNode;
}

I've not tried this (not sure if findDOMNode needs to be bound or not, for example), but if this is the only issue, it might be worth trying it, then we can stick with one npm version that supports 0.13 & 0.14.

Are there any other components supporting both 0.13 and 0.14?

@mxdubois
Copy link
Author

That seems reasonable to me. I might check the major version number too, just to be future proof.

@bruderstein
Copy link
Collaborator

🎉 v1.0 support 🎉 !

Seriously though, I suspect supporting more than two versions of react is currently extremely difficult - between two versions everything should still work, just possibly with deprecation warnings, between 3 versions, all bets are off!

Be good to know if this kinda logic works in practice though.

@JedWatson
Copy link
Owner

@mxdubois I'd prefer not to branch until we're ready to commit, long-lived branches tend to create more problems than they're worth, and any project overhead cuts into effort that could be spent on better things.

Unfortunately, require('react-dom') would blow things up with Browserify because browserify doesn't evaluate conditions (it can't, really) before bundling modules. So while @bruderstein's workaround would work in a node environment, it'll be a nightmare for packagers :(

I suspect supporting more than two versions of react is currently extremely difficult

This is correct, and we'll probably want to keep react-select compatible with 0.13 for a while, since there are major changes coming to context (among other things) that will make upgrading a patchy experience until support is consistent across major community packages and developers have had a chance to fix for them.

Not sure if we'll be able to do anything about those warnings in the meantime, as much as I'd really like to 😕

@mik01aj
Copy link

mik01aj commented Aug 24, 2015

I tried it with React 0.14.0-beta3 and apart from the warning, I get an error:

image

This is the line of code that causes this (most recent piece of your code in stacktrace):

image

@JedWatson
Copy link
Owner

@mik01aj that's often the error you get when you've got two instances of React loaded in your page. Maybe check for that? Something might be depending on 0.13 and causing problems.

@mik01aj
Copy link

mik01aj commented Aug 24, 2015

@JedWatson you are right! 😮 I have no idea how you guessed it...

@JedWatson
Copy link
Owner

Only seeing that like a million times in various issues. The first few, it took a lot more effort to work out 😛

@mik01aj
Copy link

mik01aj commented Aug 24, 2015

And now I know why. The other React version comes from node_modules/react-select/node_modules/react/lib/React.js, and it was installed there by npm because it couldn't satisfy the peerDependency of react@>=0.13.3 (I have React 0.14.0-beta3 in the main package).

@mik01aj
Copy link

mik01aj commented Aug 24, 2015

Ah peerDependencies... Is there anything we can do to prevent this installation of unneeded, second instance of React? (e.g. require version >=0.13 <0.15 or so). Imho React 0.14, despite being in beta, can be considered pretty stable.

@JedWatson
Copy link
Owner

I loved that tweet.

I've added a condition to package.json that allows for it (beta releases aren't covered by the usual syntax with semver) and am about to release a new version, just merging PRs that are ready to go & writing up a changelog now. If you're impatient, you can pull from master and it should work.

@mik01aj
Copy link

mik01aj commented Aug 24, 2015

It still installs react@0.13.3 because react-input-autosize, which is your dependency, also has a peer dependency of react@>=0.13.0. :(

@JedWatson
Copy link
Owner

Yup. I just published a new version of that too - v0.5.1. try again?

@mik01aj
Copy link

mik01aj commented Aug 24, 2015

Works now! Thanks!

(btw, I didn't notice that the react-input-autosize is yours as well, sorry for the 2nd ticket).

@JedWatson
Copy link
Owner

No worries. I needed it for react-select, and it seemed like it was worth publishing on its own 😀

I've just released a new version of react-select that should support react 0.14, let me know how you go.

@newoga
Copy link

newoga commented Oct 9, 2015

FYI, based on my understanding, React.findDOMNode() will stop working starting in react@0.15.x which according to facebook/react#4600 should be coming out soon. Also, if I'm not mistaken, it seems like all of react-select's uses of findDOMNode() can removed as of react@0.14.0 since this.refs.{refValue} now returns the actual dom node.

This is described here: https://facebook.github.io/react/blog/2015/10/07/react-v0.14.html#dom-node-refs

I guess one approach is that react-select@0.6.x can support react@0.13.x. and react@0.14.x with warnings. And then react-select@0.7.0-rc1 or something of that sort could be released to support react@0.14.x without warnings and react@0.15.x.

For react-select@0.7.0-rc1, I'd imagine that we shouldn't even need to import react-dom. We should only need to make the following changes:

var itemNode = React.findDOMNode(this.refs.item); // old
var itemNode = this.refs.item; // new

Not having a dependency on react-dom (or the DOM in general) would be a good win the for the project I think and could warrant a new minor release.

@damassi
Copy link

damassi commented Oct 16, 2015

There is also the ability to bundle with Webpack if Browserify is causing issues. I'd be willing to help if you need the resources. The react-dom warning is driving me mad since they now bold it in red.

@JedWatson
Copy link
Owner

Just realised I didn't close this when 0.7 was released with React 0.14 support. Doing that now :)

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

No branches or pull requests

6 participants