currently helpers like select will give you the local part of the state, but you don't have access to the rootState directly.
in thunks however, you can only access the root state directly:
// eg.
const model = {
foo: {
bar: "baz",
derivedState: select(state => state.bar + "_blubb"),
myAction: thunk(async (actions, payload, { getState }) => {
const bar= getState().foo.bar
console.log(bar)
})
}
};
i was therefore wondering if there would be some unified api, where it is always clear, how to access the local part of the state and how to access the root state?
not a big deal, but since easy-peasy is heading version2, i thought it was worth to put some thought into that :)
Thanks for raising this @macrozone - I do like the idea of state/actions being scoped at first. I am going to think of some ideas here. 馃憤
@ctrlplusb cool! I really like the api so far, it makes perfectly sense for most cases. This was the only case that was not 100% elegant. I also noticed it, when writing the test of the thunk: because in the tests, i setup the store only with the current local model. But when testing the thunk, you probably need to setup the full model (or at least in away that the path to the local model is the same as in the running app).
Great feedback, thank you 馃檹
@macrozone @ctrlplusb Since this is a milestone for v2, what about making the first parameter of both an action and a thunk be an object with local state and local actions.
const model = {
foo: {
bar: "baz",
derivedState: select(state => state.bar + "_blubb"),
myAsyncAction: thunk(async ({ actions, state }, payload) => {
const bar = state.bar
console.log(bar)
// ... async
}),
myAction: thunk({ state }, payload) => {
state.bar = payload
}
}
};
@cevr
i am not sure if its possible to modify the state directly in a thunk and if so, if we should allow it.
also it adds additional ceremony for most normal use cases of thunk.
What about passing an object to thunk() that is more or less a merger of actions and state?
const model = {
foo: {
bar: "baz",
derivedState: select(state => state.bar + "_blubb"),
setBar: action((state, payload) => state.bar = payload),
myAsyncAction: thunk(async (localModel, payload) => {
const bar = localModel.bar
const derivedState = localModel.derivedState
// some async stuff
// then use actions
localModel.setBar(newBarvalue)
// problem remains... should this be allowed?
localModel.bar = newBarvalue;
}),
}
};
Generally, redux users always see state as immutable and "read-only", so it would be clear that every property on localModel is "ready-only". But since we have _immer_ in place, people might unlearn that. Without immer, the above situation would be absolutly clear: you have to use actions in thunks to update state.
personally, i see the benefits of immer, but i also dislike that it breaks the functional-programming-paradigm that everything should be considered immutable and you never reasign stuff on an object (also you have to turn off eslint's param-reassign rule...)
Another very simple and save (but not so elegant) api would be:
myAction: thunk(async (actions, payload, { getLocalState }) => {
const bar= getLocalState().bar
console.log(bar)
})
Yeah, we shouldn't allow mutation of state within a thunk as it breaks away from Redux's model of synchronous state updates. Additionally we have to expose state via a function, which allows you to query it multiple times, in the case that any actions you may have fired from your thunk mutate the state.
@macrozone - I have been thinking something similar, but I feel like accessing local state should actually be the default. We should encourage operating within the scope of your model, and only breaking out in exceptional circumstances. The listen API provides a cleaner mechanism of cross model communication, with responsibility at the right layer.
I was thinking something like the following:
const model = {
session: {
token: ''
},
todos: {
items: [],
doSomething: thunk(async (actions, payload, { getState, getStoreState }) => {
console.log(getState().items);
console.log(getStoreState().session.token);
})
}
};
A breaking change, but easily migratable.
@ctrlplusb agree, i think that is a good solution.
you could already introduce getStoreState now and deprecate getState until v2, so devs might notice it before breaking in v2
Most helpful comment
Yeah, we shouldn't allow mutation of state within a thunk as it breaks away from Redux's model of synchronous state updates. Additionally we have to expose state via a function, which allows you to query it multiple times, in the case that any actions you may have fired from your thunk mutate the state.
@macrozone - I have been thinking something similar, but I feel like accessing local state should actually be the default. We should encourage operating within the scope of your model, and only breaking out in exceptional circumstances. The
listenAPI provides a cleaner mechanism of cross model communication, with responsibility at the right layer.I was thinking something like the following:
A breaking change, but easily migratable.