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

Failing test for avoidRouterUpdate #118

Closed
wants to merge 2 commits into from

Conversation

kimjoar
Copy link
Collaborator

@kimjoar kimjoar commented Dec 12, 2015

Hm, as far as I can see avoidRouterUpdate is actually broken:

Error: Expected [ '/', '/foo' ] to equal [ '/' ]

This api is causing us a lot of complexity — should we just remove it? Can't you basically "fake" the same thing using replacePath?

(The docs mention forceRefresh, but I've never looked at it, so don't know if that's something that will break? Does the value gained outweigh the complexity?)

I'll keep playing around with the code and tests to try to find a solution.

@jlongster
Copy link
Member

The main problem is redux devtools. It makes things complicated because we can't rely on simple heuristics that worked before.

We probably are making it more difficult for ourselves by storing info like that in the state. Doing things like that confuses devtools: devtools uses the actions to track changes across time, but we probably shouldn't be doing stuff like storing avoidRouterUpdate and changeId if we want devtools to work easily.

At the same time, it gets difficult to implement other solutions. I think we're onto something in #109 though. I think we could use the fact that the route state object changes whenever we want to call into history. Even if the path and all the properties are the same, the instance of the state will be different. It seems like we could use that, combined with some local variables to track other state for stopping cycles, for a more robust solution.

@kimjoar
Copy link
Collaborator Author

kimjoar commented Dec 22, 2015

Closing this in favour of #129

@kimjoar kimjoar closed this Dec 22, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants