-
-
Notifications
You must be signed in to change notification settings - Fork 848
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
Pass default value as second parameter for curried producer #111
Comments
I think the solution here should be to use ES6 default function parameters: const initialState = {count: 0};
const counter = produce((draft = initialState, action) => {
switch (action.type) {
case 'INCREMENT':
return draft.count += 1;
case 'DECREMENT':
return draft.count -= 1;
}
}); That super doesn't work though, and I'm having trouble figuring out why. Edit Oh I think I understand. Because This bug could possibly be fixed if The following code works currently, though it's a little more verbose: const initialState = {count: 0};
const counter = (state = initialState, action) => produce(state, draft => {
switch (action.type) {
case 'INCREMENT':
return draft.count += 1;
case 'DECREMENT':
return draft.count -= 1;
}
}) |
@christiangenco Right, it works in case of creating
If draft could be returned from producer filled with default value |
@klimashkin Oh dude you can totally just return the draft:
I think then the best pattern becomes: const initialState = {count: 0};
const counter = produce((draft = initialState, action) => {
switch (action.type) {
case 'INCREMENT':
return draft.count += 1;
case 'DECREMENT':
return draft.count -= 1;
default:
return draft;
}
}); |
@christiangenco Because of #103 the following aspect was added to immer:
The return statement return draft.count += 1; ...does not only modify the draft, but also return the result of the operation, and therefore would throw an error. So the best current pattern would be: const initialState = {count: 0};
const counter = produce((draft = initialState, action) => {
switch (action.type) {
case 'INCREMENT':
draft.count += 1;
case 'DECREMENT':
draft.count -= 1;
default:
return draft;
}
}); |
Ah damn it, now there is a problem with fall-through 😅. So maybe something like this: const initialState = {count: 0};
const counter = produce((draft = initialState, action) => {
switch (action.type) {
case 'INCREMENT':
draft.count += 1;
return;
case 'DECREMENT':
draft.count -= 1;
return;
default:
return draft;
}
}); |
Right, I tried to point out that you need to know type of the draft if it's a proxy or pure object to decide what to do - return it or just mutate. If pass default value as a second parameter to producer, we enforce proxy type and stop worrying about different type. const counter = produce((draft, action) => {
if (action.type === 'INCREMENT') {
draft.count += 1;
} else if (action.type === 'DECREMENT') {
draft.count -= 1;
}
}, {count: 0}); |
@klimashkin's code is way more compact. The extra What's the argument against passing a default value as a second parameter to producer? |
I think @klimashkin proposed pattern is a good idea. Anybody willing to create a PR? Preferably update the TS / FLow types as well :) Also, make sure in the readme that the difference between |
@mweststrate Only thing I don't know yet is how we can differentiate the two Or do I overlook something? Alternative would be to export another function which is explicitly designed for this enhancement. |
Immer only works with a base state that is either an array or an object, so
throwing on a base state that is a function (or a primitive) is perfectly
fine imho
Op di 6 mrt. 2018 om 19:57 schreef P Kerschbaum <notifications@github.com>:
… @mweststrate <https://github.com/mweststrate>
I would really like to implement that enhancement and create an PR!
Only thing I don't know yet is how we can differentiate the two produce
function calls, because if a function is used for the state argument, produce(baseState,
fn) and produce(fn, initialState) cannot be distinguished, neither by
count of arguments nor by the types of the passed arguments.
Or do I overlook something?
Alternative would be to export another function which is explicitly
designed for this enhancement.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#111 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABvGhMH_pp6fhSkm_K5gQd4pf1BWJexnks5tbtuQgaJpZM4SRaQA>
.
|
@mweststrate |
I took another approach to solving this for myself: a helper function for making redux reducers and actions. It uses immer under the hood, and lets you do things like this: const initialState = {
photos: { large: { url: "" } }
};
const { reducer, actions } = createReducerActions(
{
setLargePhotoUrl: (state, { payload: { url } }) => {
// mutate the state!
state.photos.large.url = url;
// don't return anything
}
},
initialState,
{ mutable: true }
);
const newState = reducer(initialState, actions.setLargePhotoUrl({ url })); |
…rry-fn resolve #111: allow passing initial state to curried producer function
…rry-fn resolve #111: allow passing initial state to curried producer function
In redux all reducers should have default value, since redux tree is bootstrapped with
@@redix/INIT
action.To workaround that, using curried producer as reducer, we have to write condition to check for undefined value of the draft. And we can't use argument's default value
(draft = {count: 0}, action)
, because in this case we don't know if we should return it or able to mutate proxy.But writing such check in all producers seems a little overwhelming. What if we could specify default value as a second parameter of producer, as we do it in
array.reduce
method?Following up #105
The text was updated successfully, but these errors were encountered: