Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Option to pass in a custom comparison function to detect similar thunks in queue #206

Closed
LiquidSean opened this issue Aug 10, 2019 · 14 comments
Assignees
Labels
feature 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt

Comments

@LiquidSean
Copy link
Contributor

LiquidSean commented Aug 10, 2019

Issuehunt badges

I am curious on thoughts of having a custom comparison function for thunks being added to queue. To elaborate further I would like to have more control over actions being added to the queue. What could help is being able to define a comparison function for actions that will be added to the queue. So when these actions are added a comparison function would be run to determine if a match exists. If a match exists replace current queued action with new one. This comparison is on the thunks actual parameters so IDs or other parameters. This would ensure uniqueness for items added. Thoughts on this would be appreciated.


IssueHunt Summary

liquidsean liquidsean has been rewarded.

Backers (Total: $100.00)

Submitted pull Requests


Tips


IssueHunt has been backed by the following sponsors. Become a sponsor

@rgommezz
Copy link
Owner

Hey @LiquidSean, that sounds like a good idea! We could have the default isEqual comparison function to be run, unless a custom one is provided otherwise. We could have a new config prop for the middleware factory, something like queueComparisonFn.

Happy to accept a PR for that :)

@LiquidSean
Copy link
Contributor Author

LiquidSean commented Aug 13, 2019

Here is what I am thinking for passing in a comparison Fn. Would this make sense? So you would assign the comparison Fn at the time you would assign interceptOffline property.

export const fetchUser = (url) => {
  function thunk(dispatch) {
    fetch(url)
      .then((response) => response.json())
      .then((responseJson) => {
        dispatch({type: FETCH_USER_SUCCESS, payload: responseJson});
      })
      .catch((error) => {
        console.error(error);
      });
  };

  thunk.interceptInOffline = true;
  thunk.queueComparisonFn = someFn;
  return thunk; // Return it afterwards
};

@rgommezz
Copy link
Owner

I'd prefer having a generic comparison function that applies to all the actions in the queue. You'd have then the flexibility to determine when 2 actions are the same, both plain objects and thunks. Basically providing your own getSimilarActionsInQueue

export default function createReduxStore() {
  const networkMiddleware = createNetworkMiddleware({
    comparisonFn: (action, queue) => { ... // do your comparision }
  });

  const sagaMiddleware = createSagaMiddleware();

  const rootReducer = combineReducers({
    foo,
    bar,
  });

  const middlewares = [networkMiddleware];

  const store = createStore(rootReducer, applyMiddleware(...middlewares));
}

@LiquidSean
Copy link
Contributor Author

@rgommezz would you recommend passing the comparison function into the reducer? So in handleOfflineAction this check for which comparison function will be done.

    const similarActionQueued = comparisonFn
      ? comparisonFn(actionWithMetaData, state.actionQueue)
      : getSimilarActionInQueue(actionWithMetaData, state.actionQueue);

@rgommezz
Copy link
Owner

rgommezz commented Aug 14, 2019

Yes, that sounds good!

Let's create a factory function and export it as the new API for the networkReducer.

// configureStore.js
import { createStore, combineReducers } from 'redux'
import { createReducer as createNetworkReducer } from 'react-native-offline';

const rootReducer = combineReducers({
  // ... your other reducers here ...
  network: createNetworkReducer(comparisonFn),
});

const store = createStore(rootReducer);
export default store;

Then inside the reducer we'll pass that function along to the handler:

const createReducer = (comparisonFn = getSimilarActionInQueue) =>
 (state: NetworkState = initialState, action: *) => {
  switch (action.type) {
    case actionTypes.CONNECTION_CHANGE:
      return {
        ...state,
        isConnected: action.payload,
      };
    case actionTypes.FETCH_OFFLINE_MODE:
      return handleOfflineAction(state, action, comparisonFn); // <= new arg here
    case actionTypes.REMOVE_FROM_ACTION_QUEUE:
      return handleRemoveActionFromQueue(state, action.payload);
    case actionTypes.DISMISS_ACTIONS_FROM_QUEUE:
      return handleDismissActionsFromQueue(state, action.payload);
    default:
      return state;
  }
}

What do you think?

@LiquidSean
Copy link
Contributor Author

@rgommezz I like that idea. I will try and start working on this.

@issuehunt-oss
Copy link

issuehunt-oss bot commented Aug 30, 2019

@issuehunt has funded $100.00 to this issue.


@kodeGenius
Copy link

Is this issue still open?

@ssasoglu
Copy link

When are you guys planning to merge #208 to fix this issue?

@issuehunt-oss
Copy link

issuehunt-oss bot commented Sep 19, 2019

@rgommezz has rewarded $70.00 to @LiquidSean. See it on IssueHunt

  • 💰 Total deposit: $100.00
  • 🎉 Repository reward(20%): $20.00
  • 🔧 Service fee(10%): $10.00

@issuehunt-oss issuehunt-oss bot added the 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt label Sep 19, 2019
@rgommezz
Copy link
Owner

@all-contributors please add @LiquidSean for code and doc

@allcontributors
Copy link
Contributor

@rgommezz

I've put up a pull request to add @LiquidSean! 🎉

@rgommezz
Copy link
Owner

Closed by #208

@BennyDeeDev
Copy link

BennyDeeDev commented Dec 10, 2020

If I understood right, its only possible to delete 1 item off the action Queue, would it be possible to allow an array of actions to remove.

For Example in Action Queue:

SAVE_TODO_ITEM
DELETE_TODO_ITEM

You could of course leave it in the action Queue and there will be no bugs, but I would prefer to not send it to the backend at all.

For now I can only find one Action and delete it from the Queue, I tried to return an array but that doesnt work either.

Is it currently possible to delete multiple items, or is there another feature for it?

Thank you in Advance :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt
Projects
None yet
Development

No branches or pull requests

5 participants