fix #276 #288
fix #276 #288
Conversation
…efreshes on same route fix faceyspacey#276
|
@sergio-toro is that what you had in mind for the fix? |
|
@connorsmallman I've added a special case to update only @ScriptedAlchemy / @hedgepigdaniel do you mind taking a look? |
|
Will take a look today! 🙏 |
| @@ -41,6 +41,20 @@ export default (initialState: LocationState, routesMap: RoutesMap) => ( | |||
| routesMap | |||
| } | |||
| } | |||
| else if ( | |||
There was a problem hiding this comment.
Is there a reason why this needs to be a separate if/else branch instead of adding another clause on in the condition of the first if statement line 27 such as ... || action.meta.location.kind !== state.kind)?
If the condition is getting too complex, perhaps its worth splitting it out into a helper function, e.g. shouldRefreshState? Not a big deal, it just seems unfortunate to introduce an extra case if it could be covered by the first one.
There was a problem hiding this comment.
In this case only needs to update the kind variable when the pathname and search are the same.
If we do a full update it will store on state.prev the same location, which It feels wrong because we haven't transitioned to a new route, we just need to call the thunk.
There was a problem hiding this comment.
@hedgepigdaniel Thanks for pointing, the condition was not correct. I've updated it to the expected case (pathname and query equal but kind updated)
Update state when action has kind of 'push'
Updating the state forces the current route thunk to be called again allowing a refresh of the current route.