Implementation of React Router v4

[kirill-konshin]

No description provided.

[diligiant]

@kirill-konshin thank you. I am trying to dig into your work; in a client env, I use (like recommended)

<Provider store=…>
  <ConnectedRouter history=…>

(all this being wrapped for dev purposes by react-hot-loader AppContainer ;)

In your example, you seem to reshuffle the order to

<BrowserRouter>
  <Provider store=…>
    <WrapperProvider …>

Which would, in the client environment, disconnect the Router from the provider. Are you taking care of the wiring in WrapperProvider (in effect, replacing ConnectedRouter), allowing me to just remove ConnectedRouter in the server env ?

[kirill-konshin]

Provider and Router don't know about each other at all, so order should not be important, but both must be before WrapperProvider in order to provide context to it. Did you have any issues with that? If yes, please create a separate issue, this is a pull request for next version of RR.

[diligiant]

Yes and no ;) ConnectedRouter knows about Provider (the only reason it exists really ;)

In server-side mode, ConnectedRouter is gone (I assume it's done by your Wrapper) so, in a redux context, I was wondering if you have any issue with a « swap » between WrapperPrivider and ConnectedRouter?

[kirill-konshin]

Oh, you were talking about react-router-redux. WrapperProvider must have both Provider and ConnectedRouter in context.

ConnectedRouter is gone (I assume it's done by your Wrapper)

WrapperProvider does not do anything with Router except it takes things from its' context.

[diligiant]

But in your example, <BrowserRouter> is out of the picture as create-react-server is provided with app.js which only has Provider & WrapperProvider in its "hierarchy" ??

You lost me here 🤗

[kirill-konshin]

You are looking at routes config, router itself is defined in different file: https://github.com/kirill-konshin/create-react-server/blob/master/examples/create-react-app/src/index.js.

[diligiant]

that's 0.2, I'm on your 0.3 branch for React Router 4.

[kirill-konshin]

The reason for this was some weird behavior of HMR in React Router... Technically WrapperProvider does not care about the order.

[diligiant]

Thank you. You mean I can have the same combo in CS & SS ?

<Provider…
  <ConnectedRouter…
    <WrapperProvider

that would be awesome!

[kirill-konshin]

There should be no problem.

Keep in mind that CRS does not support async routes yet, at least w/o static route configuration... that's the only reason why it's not released yet :(

Please keep me posted about your experience with 0.3.0 branch!

[diligiant]

of course. So cool ;)

[diligiant]

@kirill-konshin I'm facing an issue you might have already encountered. babel-preset-react-app is now 3.x. When up to date in the react-app, var creator = require(pathAbs); fails with either a SyntaxError: Unexpected token export or SyntaxError: Unexpected token import.

All you need is a minimal app.js (empty function) and

 "devDependencies": {
    "babel-core": "6.25.0",
    "babel-preset-react-app": "^3.0.0"

[kirill-konshin]

How is this related to this package? Maybe you should talk to authors of preset.

[diligiant]

Because as soon as babel-preset-react-app is updated to the latest version (and that's mandatory for recent create-react-app packages), even your create-react-app example suffers the same problem ;)

[kirill-konshin]

Have you reported that to authors?

[diligiant]

I would if the bug was theirs but the exception is raised only by server.js require(pathAbs). Somehow it's incompatible with latest create-react-app. I wish I knew why and how to fix it. node/babel message is cryptic and NODE_DEBUG dumps too much info for me.

I'll keep looking but I was wondering if you encountered the problem which would explain why you stick to old create-react-app versions.

[kirill-konshin]

I will look into it

[kirill-konshin]

I reproduced the error, will fix it as soon as I can. Looks like Babel no longer compiles files in certain paths, so I either have to reconfigure it or do something else.

[diligiant]

thank you!

[kirill-konshin]

Closing this PR because discussion has diverted far away from original purpose of this PR. Next time pls create separate issues for each question :)

Please see issue #3, I have a fix now, but I need to test it more and I will push it today/tomorrow.