Skip to content
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

Reaction value propagation glitch - the dirty flag is not set #568

Open
p-himik opened this issue Jun 13, 2022 · 3 comments
Open

Reaction value propagation glitch - the dirty flag is not set #568

p-himik opened this issue Jun 13, 2022 · 3 comments

Comments

@p-himik
Copy link

p-himik commented Jun 13, 2022

It's quite finicky to reproduce. In the code below, if you replace (> x 0) with (>= x 0) it works as expected.
When the breakpoint is hit, you can see in the Scope panel that a is marked as dirty but b is not and still has the previous value.

I would expect that (not= x b) is never triggered because x + 10 - 10 should be equal to x.

(def x (r/atom 0))
(def a (r/reaction (+ @x 10)))
(def b (r/reaction (- @a 10)))

(defn f [x' b']
  (let [x @x']
    (if (> x 0)
      (let [b @b']
        (when (not= x b)
          (js-debugger))
        (+ x b))
      x)))

(def z (r/reaction (f x b)))

(defn app []
  [:span (str @z " ")
   [:button {:on-click #(swap! x inc)} "INC"]])
@kennytilton
Copy link

kennytilton commented Jun 13, 2022

" if you replace (> x 0) with (>= x 0) it works as expected."

Just guessing, but x starts at zero, so the first time f runs it derefs b since (>= x 0) is true, and f starts as being dependent on both a and b.

With the test as (> x 0), initially b is not dereffed, so f is not linked to b. Reading the code we devs can see the potential dependency, but Reagent does not.

Reagent of course sees b depends on x, so I suspect that if the (js-debugger) is suppressed the code will run a second time and render again with the correct value.

(Aside: Some reactive mechanisms such as Hoplon/Javelin inspect the code to determine dependencies and would see the potential, but then they are over-depending some of the time, and missing dependencies reached by calling other functions.)

FYI, Matrix and JS MobX avoid this by having an independent means of deciding if a given reactive value is up to date. Matrix uses a serial, global "pulse" that gets incremented, in this case, each time x changes. When f decides to read b for the first time, it can see that b is out of date and on the fly recalculates b before returning a value. MobX has a different scheme (forgotten) for deciding if b is current.

Without an independent determinant of "up to date", this could be a tough fix. Again, just guessing.

@Deraen
Copy link
Member

Deraen commented Aug 25, 2022

As @kennytilton wrote, on the first reaction run it sees the old b value, and it will then run a second time with the updated value.

In this case, you can ensure your code always sees the up-to-date value by raising the @b' outside the if form. Not sure if we have some documentation that mentions one needs to be careful with conditionally dereffed vales in reactions (or component render bodies), but documentation could be better.

I tried writing notes on why it works this way several times but hit cases that I don't understand in each case. So currently I don't know if this is something we can fix.

It is probably related to reaction value being calculated (run) only if someone is watching it, and reaction only notifying (marking dirty and optionally running) its watches if it was run, if the reaction is just marked dirty, it doesn't mark the watches dirty.

@kennytilton
Copy link

@Deraen wrote: "In this case, you can ensure your code always sees the up-to-date value by raising the @b' outside the if form. "

The problem with moving the @b' outside the IF is that an unnecessary dependency is created if the IF condition is false. This is an innocuous excess if the unnecessary propagation leads to a trivial amount of processing. Perhaps worse is that Reagent programmers writing natural conditional code do need to stay on their toes in this regard.

All that said, again, there likely is no way around this, and again, I wrote an awful lot of reactive code successfully in my own framework before fixing the same issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants