KyleAMathews / coffee-react-quickstart

Quickstart for building React single page apps using Coffeescript, Gulp, Webpack, and React-Router
http://kyleamathews.github.io/coffee-react-quickstart
MIT License
254 stars 41 forks source link

Having trouble integrating React.Backbone into this version #10

Closed alanhogan closed 10 years ago

alanhogan commented 10 years ago

Hey Kyle,

I updated my code to match yours, but can’t seem to include React.Backbone into the project anymore. This worked just fine with the ~ July 11 version of quickstart.

Help/thoughts welcome (although certainly not your responsibility — sorry if this issue is in poor form)

Check out this branch to see the problem. And note that React 0.11 seems compatible with React.Backbone.

alanhogan commented 10 years ago

Looks like the hotloader is doing the breaking.

Uncaught TypeError: undefined is not a function bundle.js:924
replaceCreateClass bundle.js:924
(anonymous function) bundle.js:900
__webpack_require__ bundle.js:400
fn bundle.js:58
replaceCreateClass bundle.js:696
SUCCESS_LEVELS bundle.js:534
__webpack_require__ bundle.js:400
fn bundle.js:58
(anonymous function) bundle.js:429
__webpack_require__ bundle.js:400
fn bundle.js:58
(anonymous function) bundle.js:420
(anonymous function)

Because that’s where replaceCreateClass is coming from

alanhogan commented 10 years ago

Thoughts on approaches to resolve this are welcome.

KyleAMathews commented 10 years ago

I'm not sure why react-hot-loader would be breaking with Backbone.React. The simplest way to get things working in the mean time though is to remove react-hot-loader from webpack (I made a pull request for this: https://github.com/alanhogan/coffee-react-quickstart/pull/2).

@gaearon any idea what's wrong here?

alanhogan commented 10 years ago

Bad news. The same error occurs, just with a different stack trace.

Uncaught TypeError: undefined is not a function 
                      bundle.js:475
(anonymous function)     bundle.js:475
__webpack_require__      bundle.js:400
fn                      bundle.js:58
(anonymous function)     bundle.js:446
__webpack_require__     bundle.js:400
fn                      bundle.js:58
(anonymous function)     bundle.js:427
__webpack_require__     bundle.js:400
fn                      bundle.js:58
(anonymous function)    bundle.js:420
(anonymous function)
alanhogan commented 10 years ago

Is there some sort of require-order issue here?

KyleAMathews commented 10 years ago

I dunno. I'd have to look more closely at your fork. What's at line 475 of the bundle.js?

alanhogan commented 10 years ago

Line 475:

    module.exports = React.createBackboneClass({
alanhogan commented 10 years ago

Note that I require react.backbone like this: RB = require('react.backbone')

and I don’t get the error that createBackboneClass is undefined if i run this:

    module.exports = RB.createBackboneClass({

(RB instead of React) but then it doesn't create a real React component. Yesterday I got weird errors when I tried this nonstandard method of creating a React-Backbone class. Today I get this specific one:

Uncaught Error: Invariant Violation: The handler for Route "hello" must be a valid React component

Which sounds about right…

alanhogan commented 10 years ago

Not that any of that really helps me understand what’s going on…

alanhogan commented 10 years ago

Also: I tried to run this line:

RB.createBackboneClass.call React, {render: -> ... }

and I got the same error as this

RB.createBackboneClass {render: ->  ... }
KyleAMathews commented 10 years ago

Ah, figured it out. It's react-router (which I added since last time you looked at this) that's throwing that error. The module is checking if HelloWorld is an actual React component and so is rejecting your Backbone React component. File an issue with them and ask them to be less strict.

gaearon commented 10 years ago

@KyleAMathews Can you please explain it again? Why did hot loader break things?

Also, createBackboneClass is pretty much a wrapper for createClass so doesn't it return a valid component?

gaearon commented 10 years ago

The reason replaceCreateClass was in stack trace is because all module's code is being wrapped in this function. If you have an error right inside a module definition, it will appear to be from replaceCreateClass. Maybe it makes sense to make function anonymous to avoid getting people confused.

gaearon commented 10 years ago

I had weird NPM issues getting coffee-react-quickstart to run so I can't really check your branch.

However I just tried to reproduce it in a plain JS project, and React.isValidClass returns true for React.createBackboneClass result. So React.Backbone is not to blame either.

Now, could it possibly be that your react.backbone gets a different React than your app's code? Maybe you ran npm install inside node_modules/react.backbone or something, and react.backbone got its own react there? This could explain React.createBackboneClass being undefined after requiring react.backbone, as well as the failing constructor check (since React1 thinks React2's descriptor is not a real descriptor).

gaearon commented 10 years ago

I would try removing both react and react.backbone from node_modules and doing an npm install. Then check only one React is installed and it is in root node_modules. Then try again.

alanhogan commented 10 years ago

@KyleAMathews I’m catching up on everyone’s helpful comments, but it occurs to me that perhaps the CoffeeScript compiler's behavior of wrapping everything in a safety closure wrapper (IIFE) is causing some unintended side effects, such as a React somewhere not being global (and thus things blowing up only in the most subtle way when, as @gaearon suggests, two versions of React are being included…)?

alanhogan commented 10 years ago

screen shot 2014-07-30 at 10 51 50 am :confused: unexpected.

gaearon commented 10 years ago

@alanhogan When using Webpack, you shouldn't have global React at all unless you export it manually.

The React that react-backbone is using probably comes from its own node_modules. This should no longer happen when react is defined in peerDependencies in backbone.react, but for now you can just rm -rf backbone.react/node_modules and see it if it helps.

In other words, what you have:

--- react@0.11
--- backbone.react
--- ---- react@0.10

and you want just

--- react@0.11
--- backbone.react
alanhogan commented 10 years ago

@KyleAMathews Webpack is including 2 versions of React. Excerpts from bundle.js:

// ...
    React = __webpack_require__(19);
// ...
    var React = __webpack_require__(5);
// ...
    React.version = '0.10.0';
// ...
    React.version = '0.11.1';
gaearon commented 10 years ago

@alanhogan This is because there is react.backbone/node_modules/react. Webpack just uses what is closer to where it is used. Remove react.backbone/node_modules and you'll be fine for now.

alanhogan commented 10 years ago

@gaearon Re global React: Right, okay. React is indeed exported manually to let the React Chrome extension do its thing: https://github.com/KyleAMathews/coffee-react-quickstart/blob/master/src/scripts/router.cjsx#L5

Re: removing react.backbone/node_modules — thanks, will try now.

alanhogan commented 10 years ago

clay's move of react from dependencies to peerDependencies has solved this problem!

alanhogan commented 10 years ago

Side note, this seems to have restored the React Chrome Extension functionality. I think it was getting confused by the different React versions at play

KyleAMathews commented 10 years ago

Awesome! Thanks for your help @gaearon figuring this out.

gaearon commented 10 years ago

Cool, after all peerDependencies are not evil as I thought first time I bumped into them.

KyleAMathews commented 10 years ago

@alanhogan shall we close this issue?

alanhogan commented 10 years ago

rockc