Adapt for react router

[onumossn]

https://github.com/onumossn/serialize-query-params/blob/master/src/updateLocation.ts#L77 breaks routing for react router as query is not kept in sync with the url, so if a link is used to change the query and then a pushIn query is set using a setter provided by this library, many of the params will revert to the last time a set call was made.

[pbeshai]

Can you put together either a codesandbox and/or (ideally) a test that demonstrates the issue? It seems a simpler solution is just not to read the query object at all instead of adapting for react router. Also which version of react router are you using?

[onumossn]

Will get the test in sometime tonight.

The thing I was concerned about removing the query from https://github.com/onumossn/serialize-query-params/blob/master/src/updateLocation.ts#L77 is that I would be worried that some routers or some users are actually using it and in that case it might be a breaking change and may need a major version release. If that is ok, I totally agree that changing it in the underlying library is better.

I am using react router 5.2.0.

[onumossn]

https://codesandbox.io/s/nervous-waterfall-pyx3b?file=/src/App.js

1. click a=b
2. click setNum
3. click a=c
4. click setNum

The expected query should be ?a=c&num=2, but its ?a=b&num=2. The issue seems to be more cause I am spreading location to ensure that the next location has everything else the same.

[pbeshai]

After further investigation I see we are adding query to the location ourselves: https://github.com/pbeshai/serialize-query-params/blob/master/src/updateLocation.ts#L60 to support a rarely used router (found router). If I recall correctly, it may have also been provided by older versions of react router. I believe just removing do believe that this bug can be fixed by not reading from the query property: https://github.com/pbeshai/serialize-query-params/blob/master/src/updateLocation.ts#L76-L77 I'm fairly sure that was just an optimization to avoid re-parsing the URL unnecessarily. If you can simplify your PR to that I'll happily merge

[pbeshai]

Eerr I guess that's in serialize-query-params. I'm going to close this and if you don't mind opening there, we can do that or I can just update it myself. Up to you.

[pbeshai]

This is up in v1.2.0 now, thank you