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

Does this work with React 0.11.1? #26

Closed
alanhogan opened this issue Jul 28, 2014 · 16 comments
Closed

Does this work with React 0.11.1? #26

alanhogan opened this issue Jul 28, 2014 · 16 comments

Comments

@alanhogan
Copy link

I had some issues trying to use this with React 0.11.1, but I’m not sure if the problem was me or a version mismatch.

mountComponent is complaining it can't find this.props when I use createBackboneClass. (Note also that React.createBackboneClass no longer seems to be defined; I had to import react.backbone as RB and then call RB.createBackboneClass.

screen shot 2014-07-28 at 4 37 26 pm

@clayallsopp
Copy link
Owner

Hm, I ran our tests against React 0.11 and they pass

Did you try running your project against React 0.10 to verify that 0.11 causes the issue? There were also some breaking changes in that release, which might also be affecting your project

@alanhogan
Copy link
Author

Right, thanks, Clay. I previously looked at breaking changes and didn’t see any obvious culprits. I am trying the most basic usage of React.Bacebone possible. I am starting from the coffee-react-quickstart project, however.

Anyway, if you feel generous, check out this branch (then run npm install && cult watch and browse to localhost:8080) to see the problem.

@alanhogan
Copy link
Author

If I try React.createBackboneClass, the error message is as follows:

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)

@alanhogan
Copy link
Author

Hey, you know what, it looks like it has to do with the hotloader that comes with the quickstart.

@alanhogan
Copy link
Author

Thoughts on how to resolve this are welcome

@alanhogan
Copy link
Author

But it’s clear at this point the bug does not lie with react.backbone

@alanhogan
Copy link
Author

Update: the hotloader does not seem to be to blame, after all. See this update

@alanhogan
Copy link
Author

Think there may be some sort of require-order issue here?

@alanhogan
Copy link
Author

See also: remix-run/react-router#146

@markijbema
Copy link
Collaborator

In that issue it's suggested the dependencies aren't specified the right way. Personally, I use this plugin via bower, using the rails asset pipeline. If any of the other dependency management systems is having a problem, I'd be happy to merge a pull-request, but I can't test/verify these errors myself (and given the order of pr's, I suspect @clayallsopp isn't using AMD/... either).

Imo it's really sad how this problem is currently solved in JS-land, you have to support multiple systems, but of course you're not using them (all) yourself. I would be very happy if at some point we get one system everyone uses :)

@gaearon
Copy link

gaearon commented Jul 30, 2014

I would be very happy if at some point we get one system everyone uses :)

There's a "help everyone converge on NPM" movement.

In this case, it looks like a rough edge of NPM: your package specifies react and backbone as dependencies but it means that if client code specifies different versions of them, NPM will try its best to please everyone and give your library its own react/backbone. This makes sense for Node.js (different versions of the same package can be safely used by some of your dependencies) but rarely makes sense for the browser.

You should (probably) move react and backbone to peerDependencies since you require the client code to have React/Backbone, but don't ever want your lib to have a private copy of them.

@clayallsopp
Copy link
Owner

I've attempted to make the change in 209cb8d - can someone try that out? (ie use "react.backbone": "git://github.com/usepropeller/react.backbone#master" in your dependencies) (but tbh I'm sort of out of my element wrt npm packaging etc)

@alanhogan
Copy link
Author

Thanks, @clayallsopp! That absolutely has solved the problem! Big thanks for pointing out the exact problem and solution, @gaearon!

@gaearon
Copy link

gaearon commented Jul 30, 2014

No problem, just found the same problem with Underscore in my project, unnoticed for weeks. :-)

@alanhogan
Copy link
Author

Maybe Webpack can start warning about this

@alanhogan
Copy link
Author

Filed as webpack/webpack#385 if you'd like to chime in.

alanhogan pushed a commit to alanhogan/coffee-react-quickstart that referenced this issue Jul 30, 2014
nishp1 added a commit to nishp1/react-icon-rating that referenced this issue Aug 8, 2014
Maxim-Filimonov added a commit to play2lead/react-style-transition-group that referenced this issue Sep 29, 2015
To avoid conflicts with client react version - see clayallsopp/react.backbone#26
Maxim-Filimonov added a commit to play2lead/react-style-transition-group that referenced this issue Sep 29, 2015
To avoid conflicts with client react version - see clayallsopp/react.backbone#26
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

4 participants