Fix for history object location listener 1.0.2 - can't do a Pull Request.

[JustinMGaiam]

Hi,
I am looking to do a pull request to fix issues we are having with history object listener in redux-simple-router 1.0.2. Github will only let me do a pull request into your master branch, but that has more change then I need to make, ideas?

Here is the code I am looking to patch, pretty straight forward and it looks like it would even work in the current master.

https://github.com/GaiamTV/redux-simple-router/blob/1.0.2-history-fix/src/index.js#L103-L106

[bdefore]

@JustinMGaiam yes, I ran into this same issue and it breaks serverside rendering for redux-simple-router 1.x. I'm not sure why history 1.17 useRoutes listen looks like the following:

    /**
     * This is the API for stateful environments. As the location
     * changes, we update state and call the listener. We can also
     * gracefully handle errors and redirects.
     */
    function listen(listener) {
      // TODO: Only use a single history listener. Otherwise we'll
      // end up with multiple concurrent calls to match.
      return history.listen(function (location) {
        if (state.location === location) {
          listener(null, state);
        } else {
          match(location, function (error, redirectLocation, nextState) {
            if (error) {
              listener(error);
            } else if (redirectLocation) {
              history.transitionTo(redirectLocation);
            } else if (nextState) {
              listener(null, nextState);
            } else {
              process.env.NODE_ENV !== 'production' ? warning(false, 'Location "%s" did not match any routes', location.pathname + location.search + location.hash) : undefined;
            }
          });
        }
      });
    }

If it has a nextState it does not return a location.

[bdefore]

An example error I receive without this fix, since history.listen fires its callback with null for the first parameter, which redux-simple-router expects to be a valid location:

TypeError: Cannot read property 'pathname' of null
   at createPath (<app>/node_modules/universal-redux/node_modules/redux-simple-router/lib/index.js:110:26)
   at <app>/node_modules/universal-redux/node_modules/redux-simple-router/lib/index.js:147:13
   at <app>/node_modules/universal-redux/node_modules/react-router/lib/useRoutes.js:274:15
   at <app>/node_modules/universal-redux/node_modules/react-router/lib/useRoutes.js:120:15
   at done (<app>/node_modules/universal-redux/node_modules/react-router/lib/AsyncUtils.js:49:19)
   at <app>/node_modules/universal-redux/node_modules/react-router/lib/AsyncUtils.js:55:7
   at getComponentsForRoute (<app>/node_modules/universal-redux/node_modules/react-router/lib/getComponents.js:9:5)
   at <app>/node_modules/universal-redux/node_modules/react-router/lib/getComponents.js:28:5
   at <app>/node_modules/universal-redux/node_modules/react-router/lib/AsyncUtils.js:54:5
   at Array.forEach (native)

[bdefore]

@taion any negative side effects of doing this?

[taion]

No, this isn't right – you're listening to the wrong history object. You'll need to pass in your own history to match if you want to do this. This is why we're not even calling the route-enhanced object history any more in RR v2.

[bdefore]

@taion But for react-router 1.x, match does not accept a history, it builds its own.

[taion]

Might be better to re-implement match... this just smells wrong to me.

[JustinMGaiam]

@bdefore We are using React Engine which sets up its own History object that cannot be overridden on the client / browser. To add more complexity the only was to interact with the history object that React Engine creates is by grabbing it from context with in a React Component i.e. your component must specify "static contextTypes = { history: PropTypes.object }" then you can access this.context.history. We ended up passing this.context.history to syncReduxAndRouter. If we attempted to patch React Engine's client code to allow us to input our own History object lots of things did not work. Maybe this fix is specific to React Engine and Redux Simple Router.

[bdefore]

This workaround is causing issues triggering 404's on clientside resolving some route changes. What I'm seeing is any route changes that relate to query params.

[JustinMGaiam]

@bdefore thanks for letting me know the workaround has issues, it seems that you have created a fix in 2.2.5 for this issue. Will it be back ported to 1.0.x?

[bdefore]

@JustinMGaiam if you mean universal-redux, the 2.2.5 version was what used this bugfix, but I've removed it (and disabled serverside rendering for now) with 2.3.0 after finding this issue. UR 1.x hasn't gotten upstream changes in a while. is that what you're using?

[JustinMGaiam]

@bdefore I did not read the repo name when I clicked the link in this comment stream to so I thought 2.2.5 was a Redux Simple Router version, I did not see that you were linking to Redux Universal that is why I mentioned 1.x, please ignore that comment. Do you need anymore info from to me explore a fix for this issue that does not cause 404's?

[jlongster]

Closing until there is a straight-forward simple STR for a problem directly related to redux-simpler-router's code on the master branch.