Immer: Can immer detect draft changes and spare us from typing out scope blocks?

[drcmda]

• [ x ] Feature request: detect draft mutation
  
  
  
  • [ x ] I am willing to implement this myself
  
  

For instance

immer(state, draft => draft.count++)

The arguments:

• immer probably could easily flag the draft as mutated, in which case it shouldn't nag with warnings but simply use the draft instead of the return value.
• Having to write out scope blocks often takes more space than a shallow clone reducer.
• It's a a clear intent on the programmers side
• Using scope blocks seems a little unorthodox and i imagine it can be confusing (well, it was for me), due to nothing else in the javascript space behaving that way.

And whatever the outcome, i'd like to thank you for everything you did here and elsewhere (mobx). Your work has had such an impact and immer is possibly the best idea ever! 😀

[mweststrate]

Tricky case:

const res = produce(base, draft => draft.x = 7). What if base.x was 7 before. base is unmodified so res will become 7?

[drcmda]

@mweststrate If the draft is being accessed (draft.x = 7) it should run into the set handler regardless. I logged it, too, and it does. If it behaves like that consistently it could make it contractual: any mutation (even if it's the same value) seals the deal -> draft.

In the example above:

const res = produce(base, draft => draft.x = 7)

res would now remain { x: 7 } because objectTraps.set was called on it. And i think it also reflects the users intent, it would be quite strange if it turned out to be 7.

This on the other hand:

const res = produce(base, draft => draft.x)

should overwrite the object and yield 7.

[drcmda]

I guess this https://github.com/mweststrate/immer/pull/183 would be enough. Just had no idea it existed.

[mweststrate]

The void solution is now part of the readme as well: https://github.com/mweststrate/immer#inline-shortcuts-using-void

Beyond that I'm still not fully against, or in favor of this feature.

Pro: nice and short!
Con: sometimes returning something from a producer will have a meaning, sometimes not. Will it be clear to users when it will have meaning and when not? What if there is a lot of logic, what if in some complex conditional logic branch the draft is accidentally still being touched, but something else is returned in the end? That might lead to nasty debugging issues, because it could be quite unclear where the produced state is coming from, like in a producer that looks like;

function configureEnvironment(draft) {
    if (process.env.ENDPOINT)
        draft.myService.endPoint = process.env.ENDPOINT
    // lot of other code
    if (process.env.MOCK === "true")
       return { myService : { endPoint: null } }
}

This would become quite confusing; setting both MOCK and ENDPOINT, would in the end result in a service with endPoint being ENDPOINT and not null, which is what you would expect if you quickly glance over this code.

In the current implementation you will get an error, which has better fail fast behavior imho, as it signals the code should be improved.

I don't think saving 2 chars justifies loosing that clarity. (Personally I wouldn't use void thing for the same reason)

Feel free to thumb up or down if you think this change should be made (up) or not (down)

[drcmda]

Sure, the smaller the better. If you think using object traps to detect the change mechanism is worth it, it would be great to have that for sure.

The whole problem for us stems from a project we've made which has dozens of small updater functions, and the code stretches out so far compared to the previous version (redux reducers) that we thought maybe it would be nice if immer could help here a little. void of course is a way, though my co-workers are already looking strange because no one knew that or has used it before. 😄

[mweststrate]

Closing for now, I think the potential confusing makes it harder to track down mistakes in this, than the current fail fast behavior, and void seams a nice work-around for single line functions