Calling subscribe in a render fn clashes with reagent optimizations

[kimo-k]

Sorry for bumping an ancient thread, but I couldn't find anywhere more appropriate to discuss this.

We still can't just subscribe willy-nilly in form-1 components, because Reagent makes assumptions about the difference between render and re-render, and no amount of caching by re-frame can fix this. The only situation where it works correctly is in the (admittedly extremely common) special case where all logical branches of the render fn derefs the same, constant set of subscriptions (and this relies on caching to work without memory leaks a.k.a. zombie subscriptions).

The reason is that only the initial render, where mount happens, listens for signal derefs, and only the case when the identity of the component (not its arguments) changes triggers unmount, cleanup and remount.

As a consequence, this is wrong:

(defn bad-component
  [foo?]
  (if foo?
   ;; The mounted component will react only to the sub made in the inital render, and any other sub made in the
   ;; lifetime wil become a zombie at the end of bad-component's lifecycle.
    @(rf/subscribe [:sub/bar])
    @(rf/subscribe [:sub/baz])))

And so is this:

(defn another-bad-component
  [x]
  ; when x changes after the component is mounted, the original sub becomes a zombie,
  ; and the component will stop reacting.
  @(rf/subscribe [:sub/foo x]))

The above statements are consistent with logic and my own testing, and are evident by the copious amount of my blood on the floor from papercuts from re-frame.

I've made a quite nice generic solution for this problem, but I just wanted to ask here if I'm missing something before I open source it as a tiny lib.

Originally posted by @mortenschioler in #218 (comment)

[kimo-k]

Hey @mortenschioler, thanks for bringing this up. I'd be very interested to merge your fix, if you can provide it.

Any notes you have on the reagent implementation are also welcome. Do you think there's anything to be done upstream?

I think a related problem may have come up in our prod work, though we didn't pursue a generic solution. I did make a repro. I supposed this had to do with reagent's render impl, but didn't narrow down the exact cause.

[kimo-k]

On the other hand, I tried out your example components, but couldn't reproduce any bad behavior so far. Could you change my app to make the bug appear?

Also, just in case.. you're using the latest release versions of re-frame & reagent, right?

[mortenschioler]

Thanks for the reply! I wasn't actually expecting any. Will provide you with a re-creation of the issues that I'm seeing. I'll be terribly embarassed if I turned out to be wrong 🙈

My deps are re-frame 1.3.0 (recently superseded by 1.4.0) and reagent 1.2.0, even though re-frame 1.3.0 actually provides reagent 1.0.0. In other words

reagent/reagent 1.2.0
cljsjs/react 17.0.2-0.2-0
cljsjs/react-dom 17.0.2-0 :use-top
  X cljsjs/react 17.0.2-0 :use-top
re-frame/re-frame 1.3.0.0 :use-top
  X reagent/reagent 1.0.0 :use-top

[kimo-k]

No worries. We're doing science, so it's good to be wrong.

[mortenschioler]

Thanks for the understanding. I still don't understand how this happened, but I was just straight-up wrong:

https://github.com/mortenschioler/rf-sub-issue-repro

Somehow I had this behavior in my own code (which is confidential cause it's for work). I had this persistent issue with staleness I didn't understand, so I made a hypothesis that checked out with what I saw very consistently, and when what I could gather from the Reagent source made this logical, I just jumped the gun. The posted examples confirmed my suspicion, but I tested them sort of inside of my app instead of with a clean slate, which was dumb. When I really boiled it down removing some surrounding stuff I couldn't reproduce it, either.

The dependencies in my work code and the repro are the same, so the problem must be in my work code, probably in my view code, perhaps forgetting to deref something or whatever, or maybe in my subscription code. It's weird.

Good thing for me is now I can go figure out what's up with my own code and hopefully just adopt naïve form-1 subscriptions everywhere, which was a realy pain to not be able to do.

Sorry for taking your time and thanks for taking up the issue so promptly!

[kimo-k]

Might be worth your time to study the other repro, because this is a demonstrable problem with subscriptions and control flow which could be affecting your prod code. Basically, (when @a @b). You'd expect b to not run in some cases, and with regular reagent atoms it works how you'd expect. With subscriptions, b seems to run even when @a evals false.

[mortenschioler]

Very interesting. I'll have a look. Thx for being kind.