haxetink / tink_state

Handle those pesky states.
The Unlicense
29 stars 13 forks source link

Add State.update #80

Open back2dos opened 1 year ago

back2dos commented 1 year ago

Should add an update function to State that can access the current value without creating dependencies. Particularly useful for autorun, e.g.:

autorun(() -> {
  someDependencies;
  someState.value += 1;// will create infinite loop
});
// vs.:
autorun(() -> {
  someDependencies;
  someState.update(v -> v + 1);
});
nadako commented 1 year ago

Isn't this supposed to be bad practice tho? Why would you use this instead of the normal auto-observable?

back2dos commented 1 year ago

Updating states in a binding is generally a bad idea, yes. I'm working on the premise that people reach for it when other options are exhausted, an when they do, an awkard situation doesn't have to be made any worse.

The above example is really bad, actually. Since autorun may fire more often than necessarily expected, updates should be idempotent (and just setting a state to a specific value is, while incrementing it definitely isn't). This closer to a valid use case:

autorun(() -> {
  currentWindow.update(cur -> switch cur {
    case Offline if (!selectedOpponentIsOffline.value): None;
    case Reward if (currentReward.ends.passed): RewardExpired;
    default: cur;
  });
});

Perhaps still not a great example, but for the same inputs, this will produce the same effect.

With autorun still being new and not having that much experience with it, I'm feeling my way forward into its imperative territory. Since it's possible to just set a state, I suppose it should be no worse to update it based on the current value, so long as idempotence is maintained, which could be enforced like so:

public function update(computeNext:T->T) {
  var currentValue = Observable.untracked(() -> value);
  var nextValue = computeNext(currentValue);
  if (!this.getComparator().eq(nextValue, computeNext(nextValue))) throw 'update is not idempotent';
  return nextValue;
}