larrymyers / react-mini-router

A minimal URL router for React.js
MIT License
282 stars 38 forks source link

Support React 0.14 #46

Closed jakoblind closed 8 years ago

jakoblind commented 8 years ago

Hi,

After upgrading to React 0.14 I get the following error message:

ERROR in ./~/react-mini-router/lib/RouterMixin.js Module not found: Error: Cannot resolve module 'react/lib/EventListener' in /path/to/project/node_modules/react-mini-router/lib @ ./~/react-mini-router/lib/RouterMixin.js 2:20-54

larrymyers commented 8 years ago

I'm guessing this has to do with the separation of react and react-dom for 0.14.

I don't have a ton of time to devote to this issue immediately, so a pull request to get this library supported with 0.14 would be welcome.

molson504x commented 8 years ago

All you should have to do is require('react-dom')? Or am I mistaking something.....

larrymyers commented 8 years ago

I've tracked down where the functions from React that RouterMixin have moved to, though I'm still in the process of cleaning up all the tests and the example app. It's not a straight forward process since the team at Facebook have broken a few things from a compatibility standpoint.

ykka commented 8 years ago

Yep, @larrymyers any estimation when you might finish making it compatible? Thanks!

larrymyers commented 8 years ago

No estimate right now. There are still errors in the example app, and I haven't done any cross browser testing yet. @ykka Any help would be welcome. Just check out the react-0.14-upgrade branch.

ykka commented 8 years ago

I might do actually, I've just ditched the react-router... mini-router gives me much more flexibility I think.

I might look into it over the weekend if I find time.

bragin commented 8 years ago

react-mini-router is just great compared to the monstrous react-router.

A very simple fix is to introduce a dependency to fbjs package, where they moved the EventListener.js module to, and fix corresponding require in the RouterMixin.js like this: EventListener = require('fbjs/lib/EventListener'),

I am just testing the result, but it seems to work as expected. Please share your thoughts, maybe it's just pure magic.

UPDATE: I just found out that there is a react-0.14-upgrade branch where this stuff is going

ykka commented 8 years ago

Hey, so @bragin (@larrymyers) to me it seems to work, any chances of merging it to master soon?

ykka commented 8 years ago

Actually, I was wrong, it does seem to fail in my project. So I am not sure if it works or not.. I don't have enough time to look into it atm, sorry!

larrymyers commented 8 years ago

Todo for 0.14 Support:

jakoblind commented 8 years ago

Any progress on this?

larrymyers commented 8 years ago

The day job has me almost completely occupied right now. I'd welcome any help by having pull requests target the react-0.14-upgrade branch.

jakoblind commented 8 years ago

I have fixed the bugs in example app and made a pull request #49 . I have tested in Safari, Firefox and Chrome and it works fine!

react-mini-router is not compatible to react 0.12 or 0.13 because it has a dependency to react-dom. So I guess this would be a 2.0.0 release?

The update needed in the Readme would only be the requirement to React 0.14.

larrymyers commented 8 years ago

@jakoblind I agree, a major version bump to 2.0.0 seems like the right decision due to the new dependency on react-dom. Your pull request is merged, so I'll get things ready for release on master and get 2.0.0 pushed out to npm.

larrymyers commented 8 years ago

react-mini-router 2.0.0 is live on npm with support for React 0.14. Thanks to @jakoblind for doing all the heavy lifting.