Rendering after back/forward inconsistent with push

[dpwrussell]

Hi,

Full Test Case Here: https://github.com/dpwrussell/RLRBackForwardIssueDemo

I think there is an inconsistency in the way that redux-little-router handles some route transitions when they occur because of a back/forward.

I would expect that the rendering of these two scenarios would be identical:

1. Start at page 1
2. Click link (push) to page 2
3. Click link (push) to page 1

and

1. Start at page 1
2. Click link (push) to page 2
3. Click browser back button or a link using goBack action-creator.

However, they are sometimes not identical and it breaks the notion of rendering as a function of the route. An example I hit was that I clicked back and a selector would use the route to grab the appropriate object from the store to be used to render the new page. The problem was that the object had correctly updated to the new one based on the new route, but then the old route components would attempt to rerender before the switch from one Fragment to another. As the two objects in my case require different components to render, the result is a nasty flicker in the best case, or redux-form being initialised to nonsense in the worst.

I tried to debug this further, but I am having a lot of trouble understanding how Fragments work now.

One useful piece of information I can provide though is that it seems to have something to do with rendering children and when a mapStateToProps function is provided. For example, in the below code, Layout is what seems to cause the unwanted rerender of Child if pressing back from Child to Parent.

      <RouterProvider store={ store }>
        <Layout>

          <Fragment forRoute='/parent'>
            <Parent />
          </Fragment>

          <Fragment forRoute='/child'>
            <Child />
          </Fragment>

        </Layout>
      </RouterProvider>

This seems pretty serious as it means that rendering as a function of the state can not really be relied upon as the state has moved on, but what is being rendered (temporarily) has not.

[tptee]

Yikes! I'll be looking into this soon.

[mike-zorn]

I've run into this issue as well. I believe it is because context is used in the Fragment to track the state of the router. As a result, Fragments are not always re-rendered when the state of the router has changed. I was able to work around this by wrapping all my Fragments in connect as can be seen in the example below. This fixed this kind of issue for me because connect will pass the new router state on props to the Fragment causing it to re-render when the router has changed.

import { connect } from 'react-redux'
import { Fragment as Unconnected } from 'react-little-router'

const Fragment = connect(({ router }) => ({ router }))(UnconnectedFragment)

...

      <RouterProvider store={ store }>
        <Provider store={ store }>
          <Layout>

            <Fragment forRoute='/parent'>
              <Parent />
            </Fragment>

            <Fragment forRoute='/child'>
              <Child />
            </Fragment>

          </Layout>
        </Provider>
      </RouterProvider>

N.B. Even with this workaround in place, you'll need to use react-redux@5 because it is the first version to guarantee that things are updated as they appear in the JSX tree (see reduxjs/react-redux#416).

@tptee, I'd love to submit a PR to use connect to handle router updates instead of context in this repository, but it's not clear to me why the Fragment handles state propagation the way it does...am I missing something here? If not, I'll gladly submit a PR to fix this. BTW, thanks for this package 😸

[tptee]

Hey @ApeChimp , you weren't missing anything–that context usage was broken and kind of nuts (must've been half asleep when I wrote that). Check out #189, which implements exactly what you describe 😄

[tptee]

@dpwrussell @ApeChimp can you pull down master to see if merging #189 resolves the issue?

[mike-zorn]

@tptee I'm on vacation right now and don't have access to my normal dev environment, so I can't test with @dpwrussell's repro repo. I looked at #189 though and I'm very confident that will fix it: I had tried a much less ambitious change (just removing the context stuff from Fragment and connecting Fragment) on a fork which did fix @dpwrussell's scenario. Thanks so much for taking care of this. I'm really excited to see these changes!

[tptee]

Glad to hear! I'll leave this open until you're back and can confirm 👍

[tptee]

This is should be fixed in v14.0.0-0

@ApeChimp let me know if you still run into the issue with this release!