facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
227.65k stars 46.44k forks source link

Warn when two versions of React are used alongside #2402

Closed gaearon closed 9 years ago

gaearon commented 9 years ago

People lose hours of work debugging a simple issue: two versions of React being loaded at the same time.

https://github.com/gaearon/react-hot-loader/issues/32#issuecomment-60043061 https://github.com/KyleAMathews/coffee-react-quickstart/issues/10#issuecomment-50655116 https://github.com/gaearon/react-document-title/issues/1#issuecomment-60241045 https://github.com/clayallsopp/react.backbone/issues/26

Because there is no warning right away when this happens, they usually discover the problem through invariant violations. Some of them are descriptive (e.g. Uncaught Error: Invariant Violation: The handler for Route "hello" must be a valid React component) and I think I even saw warning that said something like "check if two copies of React are loaded", but some are really cryptic: Invariant Violation: addComponentAsRefTo(...): Only a ReactOwner can have refs....

Is there a reason why we don't want to warn right away when two copies of React are loaded?

pluma commented 9 years ago

There's probably no good way to find out if two copies of React are loaded (CommonJS modules are the antithesis of sharing a global scope, they're literally two duplicate but separate modules). But I would expect React to be clever enough to figure out that, say, a component is a React component even if it's not from the same copy of React.

gaearon commented 9 years ago

I think it's lesser evil to put some flag on window, like the flag React reads from devtools, than to spend any more time on this.

syranide commented 9 years ago

@sebmarkbage @zpao This is a commonly recurring problem that is very hard to debug, simply setting window.ReactWasHere = true and testing whether it is set would save a lot of people a lot of headache. Any objections to this? Should be preferable to just extend isValidElement (or w/e) with simple duck-typing that warns if it looks like a ReactElement but isn't... or something like that. It's free and doesn't interfere with intentionally running multiple React instances.

sebmarkbage commented 9 years ago

I think that in the future, this will actually just work. In the meantime we can warn here: https://github.com/facebook/react/blob/master/src/core/ReactElement.js#L230

gaearon commented 9 years ago

Would it catch all of the issues listed above though? Usually the problem comes up when some component or helper library loads its own version. Will we get to isValidElement in this case?

Besides, does React truly support independent Reacts on page right now? Do they not interfere in any way at all if they aren't mixed (e.g. browser event handling)?

gaearon commented 9 years ago

(To clarify, I was referring to @syranide'a comment)

syranide commented 9 years ago

Would it catch all of the issues listed above though? Usually the problem comes up when some component or helper library loads its own version. Will we get to isValidElement in this case?

I might be mistaken, but ignoring weird hacks then the only issue with multiple instances of React are when ReactElements from different instances gets mixed up. Depending on which API you feed them to, you're going to see lots of different errors though. So yeah... I think that should take care of all, or most at the very least.

Besides, does React truly support independent Reacts on page right now? Do they not interfere in any way at all if they aren't mixed (e.g. browser event handling)?

As long as you never stop event propagation or mount them inside each other it should be OK I think (but still not great though).

I think that in the future, this will actually just work. In the meantime we can warn here: https://github.com/facebook/react/blob/master/src/core/ReactElement.js#L230

Good point.

stefanomasini commented 9 years ago

Just a confirmation from someone who just flushed 3 hours of debugging time down the toilet that, yes, this issue is hard to debug, please make at least a warning. In my case I was having a weird exception (TypeError: Cannot read property 'firstChild' of undefined) caused by the fact that a component created with a different (uninitialised) copy of React was carrying a _owner = null, thereby breaking findComponentRoot().

sophiebits commented 9 years ago

We should already be warning in the case that an element from one copy of React is passed to React.render in another:

https://github.com/facebook/react/blob/d75512f211e4240a040eba4c4cb337f539281bc3/src/browser/ui/ReactMount.js#L363

I guess the cases here are when nesting elements from different versions inside each other?

stefanomasini commented 9 years ago

Yes. At least it was in my case. Nested elements created from different React versions.

joaomilho commented 9 years ago

Just my 2cents: I had the same issue today and my problem was using browserify 8.1.3 instead of 8.1.1

Hope it helps

charypar commented 9 years ago

The other issue with this is you can't reliably use the dependency injection mechanism, e.g. to change the batching strategy, while using components from multiple modules where each has it's own instance of React as a dependency. Even if they are the same version of React.

nottoseethesun commented 9 years ago

+1 ; thanks gaearon

sedge commented 9 years ago

@charypar said: "The other issue with this is you can't reliably use the dependency injection mechanism, e.g. to change the batching strategy, while using components from multiple modules where each has it's own instance of React as a dependency. Even if they are the same version of React."

This.

aearly commented 9 years ago

The main workaround many modular React components are using is to specify React as a peerDependency -- the parent module must provide React to prevent multiple copies. This is problematic because peerDependencies are on track to become deprecated because they introduce dependency hell.

JedWatson commented 9 years ago

I've seen this a lot as well, and had lots of requests to switch React to a peerDependency (+devDependency) in my packages which I'm currently resisting for reasons @aearly mentioned.

The most common frustration I've seen is when npm subtly installs two versions of React, and Browserify (correctly) includes both in the build. It can happen unexpectedly w/ package updates and breakage ensues. A warning from React that "hey, there is more than one of me on the page" seems like the most elegant way to warn developers early and direct them to an explanation of what's gone wrong / how to fix it.

It's a side-effect of npm's dependency rules and node's require behaviour that I end up with two Reacts when I just want one. Looks like things will get better with npm@3 (see npm/npm#6565) but in the meantime I agree with @gaearon about adding a "lesser evil" hack:

if (__DEV__) {
  var ExecutionEnvironment = require('ExecutionEnvironment');
  if (ExecutionEnvironment.canUseDOM) {
    if (window.__REACT_INITIALISED__) console.warn("Warning: more than one instance of React has been initialised. This may cause unexpected behaviour; see {url}");
    window. __REACT_INITIALISED__ = true;
  }
}

... there's a good argument to be made that this isn't React's problem to solve, but since it's a common problem and React can solve it, I think it would be great if it did.

sedge commented 9 years ago

@JedWatson A solution I found with browserify was to alias all require() calls like so (example shown with grunt):

    browserify: {
      dev: {
        files: {
          'public/app.js': ['react/index.jsx']
        },
        options: {
          alias: [
            "react:react", "React:react"
          ],
          transform: [babelify,reactify]
        }
      }
    },

so it isn't impossible to fix, but I'd make the argument that since this is such a common issue it needs to be highlighted more clearly in the React docs.

gaearon commented 9 years ago
robertknight commented 9 years ago

I guess the cases here are when nesting elements from different versions inside each other?

@spicyj when using browserify it can happen just when requiring two different versions of React because of the way it handles deduplication. Requires with the same path and code between the two versions can end up getting de-duped. Modules where the code/require path was changed between versions get deduplicated and others don't.

So I encountered this:

  1. ReactDOMSelect (from React v1) is evaluated and it mixes in ReactBrowserComponentMixin as part of its spec
  2. ReactInjection.Class.injectMixin(ReactBrowserComponentMixin) is evaluated and it adds ReactBrowserComponentMixin to all classes subsequently created by ReactClass.createClass (shared between React v1 and v2 because it didn't change between the two versions of React)
  3. ReactDOMSelect (from React v2) is evaluated and it tries to mix in ReactBrowserComponentMixin twice, once from ReactDOMSelect's own spec and once from the global injected mixin list.

As long as modules can be stateful, this approach to deduplication seems horrifically unsafe. I'll file an issue with Browserify itself, I haven't yet verified if Webpack would have the same issue. Either way - I think it would make sense to handle it specifically in React given how it can trip newcomers up.

sophiebits commented 9 years ago

Reverting #3580 for now (in #3646) as we sort out some problems.

mrob11 commented 9 years ago

THANK YOU for this discussion. I was battling with an Invariant Violation bug for half the day yesterday and then came across this ticket this morning. My setup was using a browserify-shimmed version of React and react-router loaded from CDN. After reading this discussion I just opted to use the npm versions of these libs and the error went away.

This is my first React project so debugging this problem was a pretty big headache. Thanks again for this discussion as it lead me to figure out the problem and move forward.

olavk commented 9 years ago

Leaving details of my case for future searchers: I got the error "Invariant Violation: Could not find the drag and drop manager in the context of Card. Make sure to wrap the top-level component of your app with configureDragDropContext" Also turned out I had two React versions loaded. The problem was Webpack. I was using "npm link " for one of my packages, which also depends on React. Seems webpack has problems handling symbolic links, or it's my fault configuring it improperly. Anyway, the problem was fixed by installing the package into my local node_modules directory.

gaearon commented 9 years ago

@olavk Thanks for this, I'm gonna add it to the relevant explanation gist for React DnD.

mathieumg commented 9 years ago

Would like to have this too. Haven't figured out a way to get GitHub cc'd (rather than bcc'd) without explicitly participating, sorry for the noise.

joeybaker commented 9 years ago

@mathieumg There's a button :)

mathieumg commented 9 years ago

@joeybaker I think that still doesn't CC you but only BCC, sadly. Moreover, I'm watching the react repo already, but I have a different inbox for when GitHub CCs me.

chrisirhc commented 9 years ago

Fyi: Ongoing PR #3332

sophiebits commented 9 years ago

Fixed by #3332, hopefully.

gaearon commented 9 years ago

If it helps the next person googling,

"Uncaught TypeError: Cannot read property 'hasOwnProperty' of undefined"  
"checkAndWarnForMutatedProps @ ReactElementValidator.js"

is yet another sign of this problem.

ameensol commented 9 years ago

HOURS_SPENT_HERE+=4 and counting... I've got this one:

Uncaught TypeError: Cannot read property 'firstChild' of undefined

I'm looking everywhere for the sneaky react twin, but no luck.

find ./ -name 'react' doesn't show any duplicates

The other possibilities I've read about are double-loading the react script, webpack double-bundling, or react/addons causing trouble, so those are what I'm working on now.

Insidious.

syranide commented 9 years ago

@ameensol As you've hopefully set up package.json correctly, then simply remove node_modules and reinstall everything, that usually fixes it unless there's some incorrectly configured module.

ameensol commented 9 years ago

@syranide I tried to git clone the repo and reinstall, but that wasn't the issue. Eventually I noticed that the error was only happening after the first navigation (similar to this other issue) and started taking apart my navigation menu. Turns out that for some reason putting a <Link/> from react-router inside a <MenuItem/> from react-boostrap was causing react to go nuts. I posted my findings on Stack Overflow.

ameensol commented 9 years ago

I think it also might have had something to do with my not using the <Root/> generated from react-router as the root of my component hierarchy. I was wrapping it in a higher level-component that accepts my flux instance and passes it on as context.

Isolating the issue into this repo gave me the following error:

Uncaught Error: Invariant Violation: ReactMount: Root element ID differed from reactRootID.
sophiebits commented 9 years ago

The nested links thing is due to invalid HTML and we have a new warning for that in 0.14.

gaearon commented 9 years ago

The nested links thing is due to invalid HTML and we have a new warning for that in 0.14.

By the way I got that new warning a few days ago and it's beautiful, great work. :+1:

tnrich commented 9 years ago

I'm getting this error:

Uncaught TypeError: Cannot read property '_currentElement' of null

when using npm link to require a module I'm working on. I can't be 100% sure that it's caused by having multiple versions of React, but the error does go away when I just paste the npm module directly into my app instead of using it via npm link.

I don't get this error when using my npm module the normal way (react is a dev dependency so I don't think it gets installed in node modules). Has anyone else run into this issue and come up with a fix?

Thanks!

EvHaus commented 9 years ago

I got this error while running 0.14-rc1 and 0.14-beta3 together

Uncaught TypeError: Cannot read property 'MUST_USE_ATTRIBUTE' of undefined
(anonymous function)    @   HTMLDOMPropertyConfig.js:17
zpao commented 9 years ago

Please do not report random problems in this issue. File new issues.

aphillipo commented 8 years ago

Seriously. This has taken me 3 days to figure out that my random error about Scrollbar.js unmounting the _mouseMoveTracker twice in fixed-data-table is caused by two reacts...

https://github.com/facebook/fixed-data-table/issues/302

This is not good that this bug seemingly can't be fixed? Why can React not just put the version number on the window object and never add itself again if it's there already? This is better than the hours and hours wasted by many people with this!

jimfb commented 8 years ago

@aphillipo I thought we fixed this in 0.14 (https://github.com/facebook/react/pull/3332); is it not displaying a warning?

aphillipo commented 8 years ago

Yep, I just upgraded to 0.14.2 everywhere (including inside the module) and somehow deleting react from node_modules/react-datatable/node_modules/ fixed my bug! (see here https://github.com/aphillipo/react-datatable).

Maybe there is something wrong with my webpack config? It's weird that this fixed it. Is this having two of the identical version included a subtle variant on this bug?

syranide commented 8 years ago

@aphillipo Part of the problem is react-datatable having react as a non-peer dependency (cc @pieterv) which can cause NPM to fetch a separate react installation for just that module.

aphillipo commented 8 years ago

Okay - thanks @syranide - I'll try adding this as a peer dep, however when I want to run the build for my module if developing the datatable from within my main project, what is the best way to accomplish this without issues?

ericsoco commented 8 years ago

@aphillipo you might try upgrading to npm@3, then rm -rf node_modules; npm install. npm@3 has improvements in how it handles dependencies and is better about avoiding multiple instances of a given package, since it installs with a flat structure instead of nesting dependencies within packages.

This, along with moving react from dependencies to peerDependencies in a library I am writing, solved my similar problems.

Diokuz commented 8 years ago

Another 4 hours lost.

My webpack config now is:

  externals: {
        'react-dom': 'React.__SECRET_DOM_DO_NOT_USE_OR_YOU_WILL_BE_FIRED',
        react: 'React'
  },
sophiebits commented 8 years ago

@Diokuz Can you say what symptom you experienced?

Also, if you are using the prebuilt react.js I would recommend using the prebuilt react-dom.js as well and using 'ReactDOM' in your webpack config since the secret property is unstable and will change in the next version (hence its name).

Diokuz commented 8 years ago

@spicyj yes, the problem was in prebuild version of React. The second instance of React came from require('react'). It did work somehow until React 0.14 migration. And even more: It work after partial code removal! This lead me to wrong way of long debugging.

I agree, this is my fault. But I still expect something like:

throw new Error('You are using two React instances simultaneously!!!11 The most common mistakes here: 1) using prebuild React 2) misconfiguring webpack 3) ... etc');

every time, instead of rare warning:

console.error('Only a ReactOwner can have refs');

after which I spent hours to find out whats wrong with my refs...

It seems like it is very common problem, and it is have to be more detailed and have better documentation.

JohnnyZhao commented 8 years ago

same issue with webpack and react.js loading manually.

mnquintana commented 8 years ago

@gaearon @spicyj Is it still an issue to load more than one version of React on a page, even if those instances are managing totally separate DOM trees and aren't being explicitly instantiated as globals? Like if, say, I had a package with ComponentA and ComponentB as dependencies, and they both depended on different versions of React, with npm as the dependency manager, could they be used together on one page?

sophiebits commented 8 years ago

@mnquintana No, that should work now if both are on a recent version of React.