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

Feat: Option to pass in custom comparison function #208

Merged
merged 23 commits into from Sep 19, 2019

Conversation

LiquidSean
Copy link
Contributor

@LiquidSean LiquidSean commented Aug 16, 2019

#206

Motivation

This feature will allow users the option of passing in their own comparisonFn to comparing thunks or actions in the queue. Example of a use could be creating a comparisonFn to compare arguments of this thunk to other thunks in the queue to determine if adding is necessary. Part of my motivation for this, is the application I am working on. I have thunks that are hit multiple times but with different args and sometimes the same record but just a different value for a property. For a scenario like that I would want to add the thunks that are all unique as in updating or modifying different records. For the one that is modifying the same record I would want to replace the similar queued item with it.


IssueHunt Summary

Referenced issues

This pull request has been submitted to:


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

@LiquidSean
Copy link
Contributor Author

@rgommezz could you restart the build? Not sure why the build failed.

Copy link
Owner

@rgommezz rgommezz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @LiquidSean, fantastic work! 🏅

I've left some comments, nothing major.

Also, we need to update the documentation and the API.

For the API, we need to export the new factory function on the entry point, as well as maintain the previous reducer export, using the factory function, so that we are backwards compatible.

I am happy to help with those two, so just lemme know and once we are done code wise, I'll make the changes on your branch :)

test/reducer.test.js Outdated Show resolved Hide resolved
test/reducer.test.js Outdated Show resolved Hide resolved
test/reducer.test.js Outdated Show resolved Hide resolved
Fix test cases to check shape and expected length of the actionQueue
@LiquidSean
Copy link
Contributor Author

@rgommezz for maintaining backwards compatibility is this what you are looking for in the entry point file -

  get reducer() {
    return require('./redux/reducer').createReducer();
  },
  get createReducer() {
    return require('./redux/reducer').createReducer;
  },

@rgommezz
Copy link
Owner

rgommezz commented Aug 28, 2019

For consistency, let's rename the file to createReducer and make that the default export.

Then on the API side, just do:

  get reducer() {
    return require('./redux/createReducer').default();
  },
  get createReducer() {
    return require('./redux/createReducer').default;
  },

@LiquidSean
Copy link
Contributor Author

@rgommezz I have made changes to the documentation. I think I have everything needed.

@rgommezz
Copy link
Owner

Thanks @LiquidSean, I'll take a look this weekend!

Btw, I requested a bounty request on IssueHunt so that you can be rewarded for your work 💯 I encourage you to create an account, just follow the steps stated here

@LiquidSean
Copy link
Contributor Author

Hey @rgommezz have you gotten a chance to look my changes over?

Copy link
Owner

@rgommezz rgommezz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes @LiquidSean! I think we are getting closer, but we need to improve the docs. Sorry if I misled you with something.

If you feel it's easier, I could jump into your fork and make the latest touches to the documentation. Up to you :)

docs_3x.md Outdated Show resolved Hide resolved
docs_3x.md Outdated
@@ -139,7 +139,13 @@ Note: since this component will re-render its children every time its parent's p
There are 3 features that this library provides in order to leverage offline capabilities in your Redux store: a reducer, a middleware and an offline queue system. You can use all of them or just the ones that suits your needs.

### Network reducer
A network reducer to be provided to the store.
A network reducer to be provided to the store. The reducer can take an optional comparisonFn. If no function is provided default comparison function will be used.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That introduction should be placed at the beginning of the section here.

Usage

The library exposes a simple reducer or a factory function to tailor it a bit more to your needs.

Basic reducer

This reducer will use a preconfigured comparison function for the offline queue. Please check the docs here we should link to a new section inside offline queue where we explain the default behaviour, which is lodash.isEqual for plain actions and thunk.toString() for thunks/functions. We should basically explain there that If a similar action already existed on the queue, we remove it and push it again to the end of the queue

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

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

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

Factory function

This may be useful if you are using the offline queue and would like to have more control on how it behaves, particularly when determining whether an action already exists on the queue. That's why we expose a factory function as well, so that you can provide your own comparison function. You'd use it in the next way:

// configureStore.js
import { createStore, combineReducers } from 'redux'
import { reducer as network } from 'react-native-offline';
import { comparisonFn } from './utils';
const rootReducer = combineReducers({
  // ... your other reducers here ...
  network(comparisonFn),
});
const store = createStore(rootReducer);
export default store;

The comparison function receives the action dispatched when offline and the current actionQueue. The result of the function will be either undefined meaning no match found or the action that matches the passed in action.

function comparisonFn(
  action: ReduxAction | ReduxThunk,
  actionQueue: Array<ReduxAction | ReduxThunk>,
): ?(ReduxAction | ReduxThunk)

src/redux/createReducer.js Outdated Show resolved Hide resolved
test/reducer.test.js Show resolved Hide resolved
test/reducer.test.js Outdated Show resolved Hide resolved
@LiquidSean
Copy link
Contributor Author

@rgommezz I have made a few changes to the documentation. If you don't mind jumping in to update the remainder of the documentation that would be great. 👍

Copy link
Owner

@rgommezz rgommezz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work! 👏 Just provided the last touches to the docs.

This is ready to be shipped. Please rebase with master first and we'll finally merge it!

@rgommezz rgommezz merged commit de83e25 into rgommezz:master Sep 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants