solid icon indicating copy to clipboard operation
solid copied to clipboard

`batch` does not prevent effects from running for signal values that revert to their value at the start of the `batch` call

Open zzzachzzz opened this issue 3 years ago • 1 comments

Describe the bug

Within a batch call, when setting a signal's value to a new value, and then back to the original value, effects are still run for the "changed" value.

This could be desired behavior, but I think an argument could be made for the behavior I'm expecting here.

The playground I've linked is a contrived example. I encountered this when trying to batch a couple store actions together, that involved this scenario of a value being changed, and then changed back to the original value at the start of the batch operation.

Your Example Website or App

https://playground.solidjs.com/anonymous/157e35f6-d812-4798-8222-082d559eec5f

Steps to Reproduce the Bug or Issue

  1. Go to the playground link
  2. Switch to the Console output in dev tools
  3. Click the 'Click' button
  4. Despite the final value of num() being unchanged at the end of batch(...), the effect is run, and the "new" value of num() is logged to the console. memoNum() naturally does not have the same issue.

Expected behavior

I expect changes to signal values within batch to be "batched", with only the end result being considered in determining what changed and what effects to run. Note that when calling setNum(0) when num() is already 0, effects are not rerun.

Sorry I know this is probably an inaccurate expectation of what batch should do. The docs description doesn't make a lot of sense to me:

# batch Holds executing downstream computations within the block until the end to prevent unnecessary recalculation. Solid Store's set method, Mutable Store's array methods, and Effects automatically wrap their code in a batch.

Screenshots or Videos

No response

Platform

N/A

Additional context

No response

zzzachzzz avatar Dec 12 '22 19:12 zzzachzzz

Yeah hmm... It's because the queuing happens on change at which point the value has changed, and then changing back sees it in the queue, but it still gets queued once although nothing changed. This is a result of the optimization I did when I removed older batch behavior because it no longer keeps a separate queue of signal changes to apply later because it applies them immediately. Hmm.. at the point of application(immediate) we lose that information of whether it actually changed from the old value because signals never need to know if they are "dirty" because they don't need to be touched after being set. This is unlike Memo's which do have a dirty state because change propagates through them.

So this requires a little more consideration as a Signal would need to keep some extra information around through the iteration of the batch and then be cleaned up at the end. I doubt this is something we will address soon. But will keep it in mind for future designs.

ryansolid avatar Dec 12 '22 20:12 ryansolid