Skip to content
This repository has been archived by the owner on Oct 26, 2018. It is now read-only.

Dev Tools reset wipes out location state #207

Closed
srph opened this issue Jan 17, 2016 · 11 comments
Closed

Dev Tools reset wipes out location state #207

srph opened this issue Jan 17, 2016 · 11 comments

Comments

@srph
Copy link

srph commented Jan 17, 2016

I'm not sure if this is a bug or an issue with my code. Let me know if I'm submitting this to the wrong place.

Whether this was intended or not, please let me know.

Behavior

The state.routing.location key loses its value when the state is reset.

Demo

https://github.com/chirigarikku/redux-simple-router-2-reset-state-bug

preview

@jasonh42
Copy link

Getting this issue as well. Using Redux DevTools, if you load a page, click "Reset" or "Commit", go between between pages, then try to replay the route changes, you get:

Uncaught TypeError: Cannot read property 'action' of undefined

If you don't Reset the state, then no issue occurs during replay. I wasn't getting this issue in 1.x.

@timdorr
Copy link
Member

timdorr commented Jan 20, 2016

Looks like we need to reapply something like #6. It should go here. Anyone want to try their hand at a PR?

Edit: Whoops, that was completely the wrong link....

@timdorr timdorr changed the title state.routing.location key loses its value Dev Tools reset wipes out location state Jan 20, 2016
@srph
Copy link
Author

srph commented Jan 21, 2016

i'll attempt. just found out that 2.x exposes the whole location instead of just path. i'll see what i can do haha.

// looks like we'll need to parse the queries as well.

edit: really no idea how we can do this without a hack or something. https://gist.github.com/srph/d574716c86b68604222f#file-index-js-L80

@srph
Copy link
Author

srph commented Jan 25, 2016

Can anyone point out if the solution hack I've thought of is valid?

@jlongster
Copy link
Member

This is inherit in how this works -- we need to read the location first from history. What is actually throwing the error, is it something inside this library? I suppose user code is running with this default state. Maybe we should set the initial location to a blank object like { pathname: '' } instead?

@vabruzzo
Copy link

I couldn't get it to work by setting the initial state to a blank object.

Following @timdorr's suggestion, I've set the initial state to the window's location, string, and hash as follows:

const initialState = typeof window === undefined ? {} : {
  location: {
    pathname: window.location.pathname,
    search: window.location.search,
    hash: window.location.hash
  }
}

This has worked so far in my few tests. It seems to mimic the fix for this issue when it occurred earlier on. Rather than setting the path to the concatenation of the window.location pathname, search, and hash, it sets the location object appropriately.

@jlongster
Copy link
Member

@vga- we used to do something like that, but it doesn't work reliably. We want the initial location from the router, because the actual URL location is abstracted away. If you are using HashHistory your code breaks.

@jlongster
Copy link
Member

I don't see what could be wrong with setting the initial location to a blank object. You will have to replay the first action to actually set the URL, yes, but that is exactly what is happening when the app loads.

@timdorr
Copy link
Member

timdorr commented Feb 1, 2016

It's loosely related to #193. When the middleware is created, it gets the location via history.listen, which should return what that initial value is.

I think it should be safe to set the initialState for the reducer at that time. But this still is really tricky to test. I'm going to add a Redux Dev Tools-enabled example so we can tool around with this stuff at little easier.

@timdorr
Copy link
Member

timdorr commented Feb 1, 2016

Nevermind. I didn't know the basic example already had them 😄

@timdorr timdorr closed this as completed in a00acfd Feb 1, 2016
@srph
Copy link
Author

srph commented Feb 1, 2016

Thanks <3

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants