ctrlplusb / easy-peasy

Vegetarian friendly state for React
https://easy-peasy.dev
MIT License
5.03k stars 190 forks source link

Computed value updating one state change behind action #342

Closed McMainsLiam closed 5 years ago

McMainsLiam commented 5 years ago

Currently, I have an array of strings called currentCycles, an Action used to set this array called setCurrentCycles, and a computed value that should update every time the setCurrentCycles action is called. The computed value uses the currentCycles value to retrieve the correct data. However, the value of currentCycles always seems to be one update behind the value it should be set to after called setCurrentCycles.

I am using redux-persist, typescript, as well as easy-peasy version 3.0.2 if that helps with the problem as well. It's worth noting that if I make a change to the store state later on, the computed value gets re-computed and the value of filteredTokens is correct. Also, if I do a direct request to currentCycles through useStoreState, the values are correct. Here is the code I'm using as well as some console logs that hopefully help with the debugging process:

currentCycles: [],
setCurrentCycles: action((state, payload) => {
  // Action: should print out an array of cycleIDs, which are strings
  console.log('Action: ', JSON.stringify(payload));

  state.currentCycles = payload;
}),
filteredTokens: computed(
  [state => state.currentCycles, state => state.token.tokens, state => state.workflow.currentWorkflowID],
  (currentCycles, tokens, currentWorkflowId) => {
    // Computed: Should print out the same cycleIDs from the action, but rather are printing the cycleID from a previous update
    console.log('Computed: ', JSON.stringify(currentCycles));

    return tokens.filter(token => token.workflowId === currentWorkflowId && currentCycles.includes(token.cycleId));
  }
)

Current Values: Start:

Computed:  ["ihoisdg293jpinvzlknalksf","newcycle"]

Remove ID "ihoisdg293jpinvzlknalksf":

Action:  ["newcycle"]
Computed:  ["ihoisdg293jpinvzlknalksf","newcycle"]

Remove ID "newcycle":

Action:  []
Computed:  ["newcycle"]

Expected Values: Start:

Computed:  ["ihoisdg293jpinvzlknalksf","newcycle"]

Remove ID "ihoisdg293jpinvzlknalksf":

Action:  ["newcycle"]
Computed:  ["newcycle"]

Remove ID "newcycle":

Action:  []
Computed:  []
ctrlplusb commented 5 years ago

Hey @McMainsLiam

Have you tried disabling/removing redux-persist and seeing if the issue persists?

Is there any chance you could create a minimal reproduction on codesandbox.io for me?

McMainsLiam commented 5 years ago

Hi @ctrlplusb,

First off, thank you for your help and quick reply! It seems that if I remove redux-persist, everything works fine. I am using redux-persist version 6.0.0 as well. Here is the configuration I am using when creating my store:

import storage from 'redux-persist/lib/storage';

...

export const store = createStore(model, {
  reducerEnhancer: reducer =>
    persistReducer(
      {
        key: 'easypeasystate',
        storage
      },
      reducer
    )
});

export const persistor = persistStore(store);

Is there any specific configuration I need to use when creating my store to fix this issue? I had seen some previous issues where people white-listed only their actions/state and didn't allow any computed values to be saved with redux-persist. Do you think I would need to do something similar here? Let me know if you need any more information. Thanks again for your help!

ctrlplusb commented 5 years ago

Hey @McMainsLiam

No problem. Yeah computed properties are gonna be an issue. You would need to fire an arbitrary action after creating the store to ensure they are rebound correctly.

Good news for you however; I have been building native support for persistence in Easy Peasy. The new APIs will give you all the same features of redux-persist, however, it will be fully compatible with the Easy Peasy model approach. I already have the code written and unit tested. Writing docs and doing the TypeScript definitions at the moment.

So hold out for a little bit... a proper solution is heading your way. :)

ctrlplusb commented 5 years ago

@McMainsLiam I would appreciate it if you could try out #343 👍

McMainsLiam commented 5 years ago

Hey, it's great to hear that's in progress already! I'll make a new branch and give it a shot a little later today. I'll post the results as soon as I have them. Thanks for your hard work on the library! I can't recommend it to enough people!

McMainsLiam commented 5 years ago

@ctrlplusb Just got the chance to test it out and it's working beautifully. It took about three minutes to transfer it over from redux-persist and I'm not having any of the issues from before. I really like the ability to wrap specific parts of your tree in persist without having to manage a huge white/blacklist. Do you have any estimate on how long it will be until your native implementation of persist makes an official release? Thank you again for the quick feedback and for sharing that so quickly! Is there any way I can throw a coffee/beer your way for the help? Let me know if you need me to beta test anything else!

ctrlplusb commented 5 years ago

@McMainsLiam no problem at all. Stoked to hear that it was easy to migrate over to and that is easier to apply. 👍

I'm happy to release this pretty soon TBH. I am waiting on some feedback from a couple of other people and will then cut a release after I have finished the todo items listed on the PR.

I appreciate your offer for a coffee/beer. 😀I've recently started an Open Collective based sponsorship program. Any "coffee" is appreciated. 😀

McMainsLiam commented 5 years ago

Sounds great on the timeline! I sent a little 'coffee' money your way. The amount of trouble easy-peasy has saved me was easily worth it. I hope it helps! Thanks @ctrlplusb!

ctrlplusb commented 5 years ago

You are a gem. Thank you so much.