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

Necessary to put complete routes in store? #57

Closed
Furizaa opened this issue Sep 17, 2015 · 10 comments
Closed

Necessary to put complete routes in store? #57

Furizaa opened this issue Sep 17, 2015 · 10 comments

Comments

@Furizaa
Copy link

Furizaa commented Sep 17, 2015

This also puts all components in the store and its even possible to break things with a cyclic dependency.

For example:

  1. A <App store={store} /> root component that receives the store using a prop (the store is created differently by client/server).
  2. <ReduxRouter>{routes}</ReduxRouter> is a child of <App>
  3. Routes get stored in state
  4. Therefore, using the _owner relation, also <App> is stored in the store (deeply nested somewhere in routes)
  5. store is stored in store because it's a prop of App. Cyclic dependency is complete.

I hope this makes any sense. It's a constructed case because store is passed to <App>. But nevertheless it breaks redux-devtools with persistState.

@gaearon
Copy link
Collaborator

gaearon commented Sep 18, 2015

Route handlers shouldn't be put into state. If this is currently the case it seems wrong. Shouldn't we be able to match any location (even persisted one) to the handlers during the rendering instead?

@gaearon
Copy link
Collaborator

gaearon commented Sep 18, 2015

(I have only passing familiarity with how this project works so forgive my ignorant comments. I'm just hoping @acdlite can help me understand this part :-)

@acdlite
Copy link
Owner

acdlite commented Sep 18, 2015

Redux React Router is pretty hands off about the state that is returned from React Router. We mostly just treat it as an opaque blob of "stuff" and then pass it to <RoutingContext>, which is a React Router component for rendering an app for a given router state.

Right now, one of the things React Router keeps in its state is an array of the currently matched routes. It does this so that <RoutingContext> knows which components to render.

It may be possible for us to sneakily remove the components property from the active routes before sending it to the store, then add them back before rendering. Now that the API is mostly settled I'll look into this soon.

@gaearon
Copy link
Collaborator

gaearon commented Sep 18, 2015

Thanks. :-)
I'm sorry if I sounded demanding or patronizing, I didn't intend to.

@acdlite
Copy link
Owner

acdlite commented Sep 18, 2015

No problem, I didn't get that vibe at all :)

@catamphetamine
Copy link
Contributor

(or maybe it's not the issue)

I'm getting this Uncaught ReferenceError: _classCallCheck is not defined error using transition middleware.
As you can see, it changes route from "/about" to "/about?" which makes no sense.
Do you know on whose side this bug is?

ROUTER_DID_CHANGE
action.payload.location =  Object { pathname: "/about", search: "", hash: "", state: null, action: "REPLACE", key: "8z8w06", query: Object } main.691ad4674c6c7b74bc79.js:41268:8
getState().router.location =  Object { pathname: "/about", search: "?", hash: "", state: null, action: "POP", key: null, query: Object } main.691ad4674c6c7b74bc79.js:41269:1
ROUTER_DID_CHANGE
action.payload.location =  Object { pathname: "/about", search: "", hash: "", state: null, action: "REPLACE", key: "6z49qp", query: Object } main.691ad4674c6c7b74bc79.js:41268:8
getState().router.location =  Object { pathname: "/about", search: "?", hash: "", state: null, action: "POP", key: null, query: Object } main.691ad4674c6c7b74bc79.js:41269:1
ROUTER_DID_CHANGE
action.payload.location =  Object { pathname: "/about", search: "", hash: "", state: null, action: "REPLACE", key: "60q4hz", query: Object } main.691ad4674c6c7b74bc79.js:41268:8
getState().router.location =  Object { pathname: "/about", search: "?", hash: "", state: null, action: "POP", key: null, query: Object } main.691ad4674c6c7b74bc79.js:41269:1
ROUTER_DID_CHANGE
action.payload.location =  Object { pathname: "/about", search: "", hash: "", state: null, action: "REPLACE", key: "xzef9j", query: Object } main.691ad4674c6c7b74bc79.js:41268:8
getState().router.location =  Object { pathname: "/about", search: "?", hash: "", state: null, action: "POP", key: null, query: Object } main.691ad4674c6c7b74bc79.js:41269:1
Unhandled rejection InternalError: too much recursion

Oh, I found the cause - it was request.originalUrl which placed a pointless question mark in the end.
And looks like this issue is not about this error message.

@bdefore
Copy link

bdefore commented Nov 11, 2015

I also ran into this issue, but the error occurs following the @@reduxReactRouter/replaceRoutes action.

The URL I'm trying to reach is: /users?page=1&metros%5B%5D=New%20York%20City&metros%5B%5D=San%20Francisco

@bdefore
Copy link

bdefore commented Nov 11, 2015

This does not occur if I go to a URL with a single metros parameter, such as: /users?page=1&metros%5B%5D=New%20York%20City

@Scarysize
Copy link
Contributor

Is this still an issues? I will close this for now.

@jeromebridge
Copy link

I'm getting this error when I put some components in the state. The component that gives me the error is decorated (may be relevant). Can anyone suggest how I can troubleshoot the cause of my issue? I saw some posts that suggest decorating the component may be the root cause of the error.

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

7 participants