Clojurians Log v2

I've made a correction for the record, as this bad code was stuck in my head. Hope it can help someone. Note that I didn't want to change the behaviour but this timer has time drifting and others weaknesses for real world use, this is a correction for educational purpose.

(defhook use-timer [initial-duration-secs]
  (let [[secs-left set-secs-left] (use-state initial-duration-secs)
        [is-running set-is-running] (use-state false)
        toggle-timer (use-callback #(set-is-running not) [])
        reset-timer (use-callback
                     (fn []
                       (set-secs-left initial-duration-secs)
                       (set-is-running false))
                     [])]

    (use-effect
     (fn []
       (when is-running
         (let [timer-id (js/setInterval
                         (fn []
                           (set-secs-left (fn [secs-left]
                                            (if (<= secs-left 1)
                                              (do
                                                (set-is-running false)
                                                0)
                                              (dec secs-left)))))
                         1000)]
           #(js/clearInterval timer-id))))
     [is-running])

    [is-running secs-left toggle-timer reset-timer]))

(defn force-2-digits [number]
  (.padStart (str number) 2 "0"))

(defui recipe-timer
  "Interactive timer component for recipe steps"
  [{:keys [duration-minutes label]}]
  (let [[running? secs-left toggle-timer reset-timer] (use-timer (int (* duration-minutes 60)))
        minutes (js/Math.floor (/ secs-left 60))
        seconds (mod secs-left 60)
        is-finished (= secs-left 0)]

    ($ :div.recipe-timer {:class (when is-finished "finished")}
       ($ :div.timer-display
          ($ :span.timer-time
             (force-2-digits minutes) ":" (force-2-digits seconds))
          ($ :span.timer-label label))
       ($ :div.timer-controls
          ($ :button {:on-click toggle-timer
                      :disabled is-finished}
             (if running? "Pause" "Start"))
          ($ :button {:on-click reset-timer}
             "Reset"))
       (when is-finished
         ($ :div.timer-finished
            "Timer finished!")))))

> I am thinking if the warning check that requires that all state vars used in use-effect to be listed as dependencies is 100% correct. Yes normally that’s what you wanna do, but is there not any situation where that might not be the case, that everything you use is a dependency? It seems that at least in this case you don’t want to make timer-id a dependency. I know I can dodge the whole problem by making it a ref You are exactly right, a proper design here is to use a ref for timer-id, since your code doesn't require any reactivity on timer-id value. So you are not dodging when using a ref here, but using the API properly.

not everything should be a piece of state, unless you need something to run reactively when a value updates

no ref here! it's an antipattern in that case. Side effects in useEffect (like a setInterval) should clean themselves in the cleanup function.

Yes, and the useEffect will clean itself, since it depends on is-running. is-running is exactly that reactive value that controls the side effect.

Yes but then timer-id should not be a ref.

want to elaborate why?

> it should not be outside of useEffect ah, yes I agree that makes sense! if the id is not needed elsewhere then it should go into useEffect, good catch

(you can look at my "correction" code, the effect only depends on is-running now)

I am still not versed about when I should call useCallback and use Memo

they are only for optimization purposes but are basically the same. useCallback produces a function to call, useMemo a value to use. See https://react.dev/reference/react/useCallback#skipping-re-rendering-of-components in my code with the current use they are useless for optimization.

@roklenarcic you should really go through react docs

I know ~nothing about Clojure and mostly joined this Slack to ask questions about Clojure on the backend. But I got nerd sniped by this. The official React docs are good, but this may also helpful if you want to understand how React useEffect hooks work under the hood since it seems like that may be useful in this context even though you are using ClojureScript (which I am not familiar with). https://overreacted.io/a-complete-guide-to-useeffect/ Also for refs: https://overreacted.io/making-setinterval-declarative-with-react-hooks/ These posts are fairly old by this point but I believe the relevant parts of React library haven't evolved much in the meantime so they should still be accurate, they're by a former React team member, and they illustrate some of the key concepts in more detail than the docs if that is helpful.

When there are new values, it means that whole let is run - there's no magic here, no custom macros. It means that the uix/use-effect form was also executed. With the (fn [] ...) capturing the new values. The hook then sees that [is-running time-left] vector at the end, checks the values there against previously supplied values, and if they differ, runs that anon fn. Don't know about uix/use-state though. Maybe the whole component is re-run and is only re-rendered if the values are different. But that would be wasteful, so no clue.

Why would the whole let be run if I call set-time-left for instance? Seems kind of a very basic unit of programming that cannot be rerun specifically here. I could see the whole recipe-timer block being rerun (which includes the let)

Something must bind time-left to its new value. Nothing other than running the whole let can do it.

Right, but the react code cannot get a handle on the let block from this

how would that even work

code like use-state cannot possibly capture the surrounding let

That's why I'd assume the whole component is being run. And I'd also assume that's why useEffect and similar hooks exist - so you can put costly computations in a hook, so that they don't interfere with rerunning the whole component function.

Ok so the whole component is rerun.

It's trivial to check - just log something in its body.

That means the hooks know somehow know which component instance contains them

Why would they care about component instances? Unless I'm missing something, all they should care about is in the last vector that's passed into them.

Ah, I guess you mean that use-effect that's used by the same instance of a component should not create new effects. Yes, that's true.

In any case the AI suggested there’s a bug in its own code:

but if what it says is true and the whole use-effect is rerun and new values are captured into anon function each time, then this bug false?

why would it have stale value?

That was the first thing that I myself noticed in your code. :D The bag is still valid. But AFAICT it's not about a stale value, it's about the effect body not being run at all when timer-id is changed.

I think that’s on purpose. The timer is reset when it’s turned off/on and time is changed, if it also ran when timer ID was changed then that would trigger itself

the body of effect is calling set-timer-id

I see. Yeah, makes sense, doesn't seem like a bug then. I myself would probably leave a comment near the last vector explicitly saying that timer-id must not be there at all, lest someone else comes and "fixes" that code. Be it a person who didn't look into the impl of the effect at all, like me, or some agentic AI that's just too happy to change everything without understanding.

Pressed LLM on this:

Thanks for catching my mistake! The closure behavior you described is exactly correct.

this is close to useless lol, just wrong information factory

Welcome to the club.

You should read React docs, it’s the same code structure in JS. They explain very well the hook concept and functional components. And yes hooks remember in which component instance they are, that’s how the component state is managed.

https://react.dev/learn/state-a-components-memory

Hm interestingly as @p-himik said that the dep to timer-id was missing, seems that react doesn’t let me use it and not declare it as a dep:

42 |     (use-effect
-----------^--------------------------------------------------------------------
 React Hook has missing dependencies: [timer-id]
Update the dependencies vector to be: [timer-id is-running time-left]
Read  for more context

so because timer-id is used, it wants me to declare it as a dependency

Ah it seems this is UIx linter that is making this warning

------ WARNING #2 - :uix.linter/missing-deps ----------------------------------- File: /Users/roklenarcic/clojure-projects/docu/src/roklenarcic/docs/recipe_widgets.cljs:83:5 -------------------------------------------------------------------------------- 80 | (or (.getItem js/localStorage storage-key) “”)) 81 | [is-editing set-is-editing] (use-state false)] 82 | 83 | (use-effect -----------^-------------------------------------------------------------------- React Hook has missing dependencies: [storage-key] Update the dependencies vector to be: [storage-key notes] Read https://react.dev/learn/synchronizing-with-effects#step-2-specify-the-effect-dependencies for more context

(use-effect
 (fn [] (.setItem js/localStorage storage-key notes))
 [notes])

your code is definitely wrong, but I don't really understand all the variables. What is duration-minutes ? Some constant or a controlled dynamic value from a parent? You may not need any use-state at all or just one. I'll tell you how to fix your code (after you answer for the variable).

ho ok it might be the time the timer has to run from the first instantiation, is that right? Then what do you want if that prop changes? Restart the timer or ignore the change?

Also for the warning, it's React itself that does this at first (might be recoded/wrapped by uix tho). You should not both use and set a state from useEffect, it feels wrong and it is in that case.

It’s a timer component cooked up by AI:

(defui recipe-timer
  "Interactive timer component for recipe steps"
  [{:keys [duration-minutes label]}]
  (let [[time-left set-time-left] (use-state (* duration-minutes 60))
        [is-running set-is-running] (use-state false)
        [timer-id set-timer-id] (use-state nil)]

    (use-effect
      (fn []
        (if (and is-running (> time-left 0))
          (let [id (js/setInterval
                     #(set-time-left dec)
                     1000)]
            (set-timer-id id)
            #(js/clearInterval id))
          (when timer-id
            (js/clearInterval timer-id)
            (set-timer-id nil))))
      [is-running time-left])

    (let [minutes (js/Math.floor (/ time-left 60))
          seconds (mod time-left 60)
          is-finished (= time-left 0)]

      ($ :div {:class (str "recipe-timer" (when is-finished " finished"))}
        ($ :div {:class "timer-display"}
          ($ :span {:class "timer-time"}
            (str minutes ":" (if (< seconds 10) (str "0" seconds) seconds)))
          ($ :span {:class "timer-label"} label))
        ($ :div {:class "timer-controls"}
          ($ :button {:on-click #(set-is-running (not is-running))
                      :disabled is-finished}
            (if is-running "Pause" "Start"))
          ($ :button {:on-click #(do (set-time-left (* duration-minutes 60))
                                     (set-is-running false))}
            "Reset"))
        (when is-finished
          ($ :div {:class "timer-finished"}
            "Timer finished!"))))))

yea it's bad, you should not learn from code by AI

Reading the code I'm pretty sure the effect is called every render (when the timer is running) so the setInterval is useless, it's used as a setTimeout

(I was not going to fix the code, but if you think it is useful, please tell me I can do it)

No need, this is just a thing AI cooked up and I am trying to understand how things work

I am thinking if the warning check that requires that all state vars used in use-effect to be listed as dependencies is 100% correct. Yes normally that’s what you wanna do, but is there not any situation where that might not be the case, that everything you use is a dependency? It seems that at least in this case you don’t want to make timer-id a dependency. I know I can dodge the whole problem by making it a ref

timer-id is not needed at all here, I mean outside of the useEffect function. You just get the id locally and clear the interval with the cleanup function (that get the id from the closure)

I won’t say 100% but usually if you have a warning it is meaningful, it also might warn you about an antipattern like here