Disable query injection when key exists already

[dr3]

When using ConnectedRouter you may have already set a query with a custom parser like this

const history = qhistory(createBrowserHistory(), stringify, parseQueryString);

<ConnectedRouter history={history}>

This PR ensures it doesn't override existing queries which may have different parsing logic

Ive published this under https://www.npmjs.com/package/@dr3/connected-react-router for the meantime :)

[josehenriquelicio]

This would solve #383 too.

Thanks, I'll be using your fork until this is merged.

[supasate]

Sorry for slow response. LGTM!

[tedpennings]

This introduced a bug. The reducer query field is only updated when it has disappeared and reappeared. In all other cases, the query field is stale and not updated, not reflecting history replace/push changes.

Here's an example: in our product a Lightstep, we use query params for the chart time window (?time_window=). We use a history replace action to change the ?time_window= URL query param, and that URL query param is the authoritative source of that state (good for link sharing). With this change, the reducer query field never updates when the query params location.search changes -- only when a user navigates to a page that does not have query params.

I think the if (location && location.query) { return location } statement should be if (location && location.search === state.search) { return location}, so it updates when the query params change, but accomplishes the goal from this PR.

Would that work for you @dr3?

---

PS: for anyone finding this, use the search reducer field instead of query and parse it with something like qs. You can do this in a reselect memoized selector to make it less expensive and avoid wasted render cycles from object equality misses (qs.parse returns a new object each time).

import { parse, stringify } from 'qs';
import { createSelector } from 'reselect';

const selectQueryParams = (state) => state.router.location.search; // state.router may be different for you

export const getURLParams = createSelector(selectQueryParams, (queryParams) =>
  parse(queryParams, { ignoreQueryPrefix: true })
);

[dr3]

Ah, apologies @tedpennings. Its been a while since I worked on router, I havent tried this out, but in my usage

const history = qhistory(createBrowserHistory(), stringify, parseQueryString);

my worry would be that when search does change, it would override the output of parseQueryString with the implementation in this package. But I agree, I think this did introduce a bug for those not using qhistory to manage its query object separately

[tedpennings]

@dr3 that's reasonable. I'm not very familiar with qhistory. Maybe we could pass a custom query string parser to createConnectRouter, defaulting to the one in this package?

Also, what does qhistory's parser do that this one doesn't? Is it a boolean/number type coercion thing? (....the worst part of working with query strings)