treora / redux-query-sync Goto Github PK

I found that using redux-query-sync with a range input was resulting in quite a bit of lag when using the slider. Any advice on how to address this or reduce the performance impact?

As reported by @riccardoscalco:

I see an unexpected behaviour(initialTruth: 'location' and defaultValue: undefined): the user can repeatedly click on the browser back arrow and it remains active.

This sounds like on initialisation, handleStateUpdate is triggered (as expected) and ends up with a newLocationSearchString different from oldLocationSearchString, and thus calls locationUpdate (=history.pushState), thus directly creating a new history entry. The question is why the state is different than expected.

One possible problem I just realised, that could be causing this: when multiple parameters are synced, actions are dispatched in sequence, and a handleStateUpdate might be triggered after each of them. After e.g. the first of two such actions, handleStateUpdate would trigger and still find an old value of the second parameter in the state, and update the location to match it. Then the second action should update it in the state and which causes a final update in the location again, so if I am not mistaken it ends up fine, but with some undesired history entries.

@riccardoscalco: It would be interesting to know if your described problem also appears if you have only one parameter in the configuration. There could well be other mistakes that cause this issue.

I should scrutinise the exact flows of actions and updates some time soon. If in the meantime anyone figures out either the problem or solution, give a shout.

If url param is bind to some dynamic store state, then it's not enough to know only the value we also need some other info to set this value to appropriate state.
The solution is simple - pass a state to an action too.

I spent a good deal of time suffering with what appeared to be a bug but turned out to be a feature. In short, if you use BrowserRouter in React Router 4 (as much of the documentation suggests) you'll struggle with redux-query-sync.

The docs here make clear that you'll need to use a common history object in order to keep everything in sync and behaving properly. I tried to do this by declaring my own history object and using it in ReduxQuerySync's function call. Then I tried to assign it to my instance of BrowserRouter but it didn't work.

Well it turns out that BrowserRouter creates its own history object and ignores the history that you pass it. It also doesn't throw an error or provide any notification that it is ignoring the property. Instead you need to simply use Router which accepts the history argument. My understanding is that there is no functional difference between BrowserRouter and Router except for the internal declaration of history in the former.

I'm not sure that this is even redux-query-sync's responsibility, but given the time that I spent troubleshooting the issue I figured attaching this issue here might save some poor engineer a few hours in the future.

Otherwise, the library is amazing and exactly the tool I needed for my project.

Scenario:

I have some initial state in my redux store that is set to an empty array: []
I set the defaultValue property accordingly to ensure that the corresponding url parameter is not visible should the initial state be equal to []

Expected behaviour:

If the initial state is indeed [] then the parameter should not be present:
defaultValue: []
state: []
http://www.test.com

Observed behaviour:

If the initial state is set to [] then the parameter is present but it's value is an empty string:
defaultValue: []
state: []
http://www.test.com?state=

Source of problem

The issue seems to stem from the value comparison in handleStateUpdate:

            if (value === defaultValue) {
                locationParams.delete(param);
            } else {
                locationParams.set(param, valueToString(value));
            }

If the defaultValue is a reference to an empty array, then the === operator will return false if it is compared to a different array instance.

Potential solution

One way to mitigate this would be to cast both values as strings: valueToString(value) === valueToString(defaultValue).

@Treora Please share your thoughts :)

This is a great library by the way!

Rather than setting the defaultValue for each parameter separately, and to have a single point of truth, I would like the default value to be inferred automatically from the store's default state (by applying the selectors to it).

Question is: what is the default state? In theory, it should be what reducer(undefined) returns. Some people might however want to consider initialState as their default, or the state after createStore has run its reducer(initialState, INIT) (here), whereas this would break in for people that let their server side provide an initialState object based on the URL query parameters (we would then use the parameters, but hide them from the URL again). One option is to leave it up to the user to provide the default state:

const store = createStore(reducer, initialState)
const stopSync = ReduxQuerySync({
  store,
  params: {...},
  // For cases where initialState already incorporated the URL's query parameter values.
  defaultState: reducer(undefined, {type: 'dummyInit'}),
  // For cases where initialState is used as the default state.
  defaultState: initialState,
  // Or cases where both initialState and the reducer's initial response to it.
  defaultState: store.getState(),
})

The ReduxQuerySync.enhancer approach would have access to reducer and store, and could do any such a step for us, though it may need an option to choose which approach to take.

The example is missing context for the store object, among other things.

Adding more context so the code works immediately would be helpful.

Thank you so much for this library. It's saved me so much hassle, and I think it's the correct way to think about state and url as a window into the state.

Currently we have a global option replaceState to say whether each change to the state should update the url by replacing it (not creating a back/forward history entry) or pushing to it (creating a back/forward history entry). Is it possible to have this option on a per-parameter basis, such that changes to some parameters don't create the entry, but some do?

Going even further, could we get the new and previous values and decide based on that whether to push or replace? So it would look something like this:

ReduxQuerySync({
    store,
    params: {
        p: {
            selector: state => state.pageNumber,
            action: value => ({type: 'setPageNumber', payload: value}),
            replaceState: (value, prevValue) => someLogic ? true : false
        },
    }
})

This might be overcomplicating things though. I can explain my use case if need be.

If this is not on the roadmap, do you have a recommended way to implement this ourselves?

Let's consider the following case:
www.test.com/catalog1?sortField=fld1&sordDirection=desc&filter='some filter'
and another one is
www.test.com/catalog2?sortField=fld2&sortDirection=asc&filter="another filter'
These are two different pathnames and they contain absolutely different info and has its own state in store.
So my proposal is to make it possible to define a pathname depended params and also keep the current way of working with not path related params.
The config can look like this:

ReduxQuerySync({
    store,
    params: {
      pathSpecific: [{
         pathname: /^www\.test\.com\/catalog1\/.*/,
         params: { 
             sortField: {
                 selector: sel1,
                 action: act1,
                 defaultValue: def1,
             },
             sortDirection: {},
             filter: {}
         }
      }, {
        pathname: /^www\.test\.com\/catalog2\/.*/,
        params: {
            sortField: {
                selector: sel2,
                action: act2,
                defaultValue: def2,
            },
            sortDirection: {},
            filter: {}
        }
      }],
      ... //other not path specific params
   },
})

I did implement this idea already because I need this behavior in my project.
What do you think @Treora ? I also thought that for not path related params it could be just empty pathname, then we don't need to separate path specific and not path specific params.

The example given by JFFar is not working anymore, someone has a copy of it and want to share it ?
[https://github.com/J-F-Far/url-redux-sync/blob/master/src/index.js]

Hi there,

One odd gotcha that I found is that the conditional here makes for confusing behavior in the following circumstance:

1. A query parameter is absent from the URL when its associated action creator is dispatched
2. The value in the state is the same as the defaultValue supplied to `ReduxQuerySync

In this situation, locationParams never has its value set, so the url is never updated.

It would be helpful if there could be an additional check here to see if the query parameter is present in the actual URL. I'm happy to put out a PR for this, if you like.

Thanks for all of your work on this!

It would help to have a fully working React example app that uses a Redux store, to make it easier to understand how to use this module in an existing project.

How would I go about unit-testing my usage of ReduxQuerySync(), or is there any way to without doing a fuller integration / acceptance test where I use a bunch of different query-parameters for the entire app?

We notice in line 121 of handleStateUpdate method, the logic of checking if a location param needs to be deleted doesn't support deep comparison

    if (value === defaultValue) {
        locationParams.delete(param);
    }

We have situation when initial value for a state is an [], and since [] === [] returns false, it doesn't delete this parameter from URL, thus leave something like ?param=, can we please add support for deep comparison so that it can also check object equality?

I have situation where a single query key is used multiple times to provide several values. The underlying URLSearchParams supports this case, but in getQueryValues the code uses only the first value.

I suggest to add a parameter option that specifies if the parameter is of array type and if so to use the URLSearchParams.getAll() for retrieving the values.

This would be really nice to have either for the entire config or for a given action. Right now if the action method returns null then an error about no property type is thrown. By allowing the action function to return null, this could enable a custom-built debounce solution. One built-in would be even better

Hey, thanks for the great library. In our app we have routes like /hello/world/#/?type=foo. It seems that the library doesn't work with this kind of url, since the values are not being read/applied to the store. Is there a way of getting it to read/write the search query after the hash?

Thanks

I was under the impression that omitting replaceState from the options would allow the user to cycle back through the selections they made using the back button, and that's sort of happening, but not the way I expected. What's happening is I can see it very quickly cycle through the previous states and then navigate back to the previous page, or if I put in a breakpoint it only cycles back one state.

store setup:

...

    ReduxQuerySync({
      store: this.store,
      initialTruth: 'location',
      params: queryParams
    });

...

queryParams:

import { moveToTab } from 'bundles/project/actions/index';

const queryParams = {
  tab: {
    action: value => moveToTab(value || 'overview'),
    selector: state => state.ui.activeTab,
    defaultValue: 'overview'
  }
};

export default queryParams;

moveToTab action:

export function moveToTab(tab) {
  return { type: MOVE_TO_TAB, tab };
}

I am using combineReducers, not sure if that would have an effect on the library's ability to step back through the previous states.

Let's say I have two query params: start & end. Is there a way to say, if start is passed in, then end must also be set, and vice versa, else unset both (or set both to default value)?

Since each param sends an individual action to the store with just that param, then the reducers only see one param set at a time, so I can't really do the check at the reducer level (unless I'm wrong about this?)

Is there some kind of solution or workaround to this?

Hello!

Firstly, thank you for working on this! It's coming in very handy.

I have a question of sorts.
I know that per RFC rules, querystrings are case sensitive.

However, in specific use cases, it would be useful to configure the enhancer params matching to be case insensitive.
In my particular case, I'm in the process of migrating from pascal case to lower case and wanted the params to match both cases.

If you're open to the idea, I would be happy to submit a PR.

We now either have to choose whether the URL params initially overwrite the state, or the state overwrites the URL, but sometimes you may want to incorporate information from both.

An example scenario: my application stores user preferences in the local storage, let's say the last viewed x and y coordinates on the map. These should also be exposed in the URL, so the user's view can be linked to, but when I visit the page without specifying an x or y, they should default to their stored values. Setting the defaultValues to the stored values is inappropriate, since it would hide them from the URL.

A solution would be to let the given URL parameters overwrite the values in the state, but for parameters that it does not specify, we let the state write its values to the URL. This only needs to happen on initialisation, after which they are kept in sync. An option initialTruth: 'merge' might be good for this, perhaps even as a default setting. (the name might have to change, as perhaps we want to also support merging in the other direction, i.e. letting non-default state values override URL params)

I would like to apply sync query to different states on different paths.

For example, let's say there are two paths.

/books?page=1
/computers?page=1

I would like to know if i can create enhancer like this.

ReduxQuerySync.enhancer({
    params: {
        page: [{
           path: '/books'
           ... // actions, selectors, etc
        }, {
           path: '/computers'
        }]
    },
})

This currently does not work in IE or Edge as they do not support Url("").searchParams. Maybe just including a polyfill will fix it?

In function handleLocationUpdate we collect actions in an array and then dispatch all of them in a separate loop. The problem is that some params can depend on another. For example, we have a query params country and currency. Currency depends on country. But in current solution if you set country to a new value, the currency in its selector will still use an old value.
To solve this problem we need 1) dispatch an action right after we get new param value 2) somehow define an order of processing params - less depended first. This can be done manually writing params in necessary order in ReduxQuerySync config.

Any thoughts?

Hey!

I opened this issues because I ran into a small hick.
URLSearchParams encodes the query string by default, so this:
date=12/04/2018&other=1,2,3,4 Becomes this: date=12%2F04%2F2018&other=1%2C2%2C3%2C

For my case, I was thinking of being able to configure this as well, since I don't want encoded url to show as much as possible in the address bar.

However, a bigger issue is this line:
https://github.com/Treora/redux-query-sync/blob/master/src/redux-query-sync.js#L132

This comparison will fail, even if the query string did not change. Because the value in oldLocationSearchString can be a decoded string and the newLocationSearchString will always be an encoded one.

I'm thinking if just always decoding the string when getting it from URLSearchParams would be a viable option? Or just encoding both and comparing them.

In my case I don't actually want the parser to replace by decoded string for an encoded one unecessarily. Which is what is happening as of now.

Cheers!

I've been working with this package (many thanks, helped me out a lot!) for a Gatsby site, and ran into some compatibility issues because Gatsby uses Reach router. I changed some things around to make Reach router play nice with your package. The changes are pretty minimal. I'd like to add those changes to your repo (after clearning everything up), if you're up for supporting both React Router and Reach router?

React Router's history.push('/thing') squashes the query params (what it calls location.search).

One can do history.push({pathname: '/thing', search: location.search}), to preserve the location.search, but an action could be dispatched just before history.push which changes the query params, so the location.search that history.push is using is now out of date, and it again squashes the query params.

It's making it really tough for me to use this package, which is otherwise dope.

Any ideas?

Nice package, btw!

Please Provide some Demo Examples.

Hi! How can I change other parts of url, not only query params? Why do you use internal browser history and do not allow to pass it as a parameter?