immerjs / immer

Create the next immutable state by mutating the current one
https://immerjs.github.io/immer/
MIT License
27.73k stars 852 forks source link

Q: Detect if draft has changes to be commited before finishDraft #455

Closed kilianc closed 5 years ago

kilianc commented 5 years ago

Is there a way to tell if a draft is in a "dirty" state?

mweststrate commented 5 years ago

No, dirtyness is only reliably determined once the producer finishes. What is your use case?

On Thu, Oct 31, 2019 at 1:07 PM Kilian Ciuffolo notifications@github.com wrote:

Is there a way to tell if a draft is in a "dirty" state?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/immerjs/immer/issues/455?email_source=notifications&email_token=AAN4NBFK73W2EERBJE24ZGDQRLKCNA5CNFSM4JHJZCO2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HVYVTGA, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBH4H6XKWDE3USYYIITQRLKCNANCNFSM4JHJZCOQ .

kilianc commented 5 years ago

@mweststrate I am trying to experiment passing out a draft of the global state to my render tree along with actions bound to these subtrees, let's say updateUser({ name: 'Paul' }) calls a commit function internally that calls finishDraft. If newState and oldState are unchanged setState will be a no-op and I won't have the chance of recreating a new draft while the old draft is closed.

kilianc commented 5 years ago
  const [state, setState] = useState<State>(initialState)
  const draftState = createDraft(state)

  const commit = () => {
    draftState._version++
    const newState = finishDraft(draftState) as Readonly<State>
    setState(newState)
  }

This is my current hack.

mweststrate commented 5 years ago

But why do you want to call setState if it didn't change anyway?

Op do 31 okt. 2019 16:58 schreef Kilian Ciuffolo notifications@github.com:

const commit = () => { draftState._version++ const newState = finishDraft(draftState) as Readonly setState(newState) }

This is my current hack.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/immerjs/immer/issues/455?email_source=notifications&email_token=AAN4NBENZZ2L3D2NXNSL4ILQRMFEZA5CNFSM4JHJZCO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECYQEBQ#issuecomment-548471302, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBBDFSP223QSFD4PZBTQRMFEZANCNFSM4JHJZCOQ .

kilianc commented 5 years ago

@mweststrate I don't want to, that's the point.

mweststrate commented 5 years ago

why forcefully increase the version then? If you finish first, you could increase the version only if the state actually changed.

On Thu, Oct 31, 2019 at 7:36 PM Kilian Ciuffolo notifications@github.com wrote:

@mweststrate https://github.com/mweststrate I don't want to, that's the point.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/immerjs/immer/issues/455?email_source=notifications&email_token=AAN4NBBWMJKEAWPBE3HNNNTQRMXVLA5CNFSM4JHJZCO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECY733A#issuecomment-548535788, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBHOZNRSAEMCEGJH2ITQRMXVLANCNFSM4JHJZCOQ .

kilianc commented 5 years ago
function useGlobalState(init) {
  const [state, setState] = useState(init)
  const draftState = createDraft(state)

  const commit = () => {
    const newState = finishDraft(draftState)
    if (newState !== draftState) setState(newState)
  }

  return [draftState, commit]
}

This will result in a "draft already closed" error if I call commit without changing anything and then call it again. That's why I am using the _version bump as hack so I always have at least one change!

mweststrate commented 5 years ago

I think the pattern you are using above is a very dangerous one, it is recommended to not hand out drafts left and right and have components hold on to it over time. In that case you could just use well use mutable objects int he first place and skip immer entirely :) Using drafts and immutable states now just increases complexity: you need to make sure you propage the latest draft with the latest state, and as soon as any component bails on with whatever optimisation that avoids propagating state, you get into a mess because those things will have an old draft.

It basically relates to the principles outlined here: https://egghead.io/lessons/react-write-asynchronous-producers-in-immer-and-why-you-shouldn-t