reagent-project/reagent

shadow-cljs emits a warning when using with-let

timmw opened this issue · 4 comments

timmw commented

Similar to this issue reported a while ago.

When using with-let I get the following warning.

Cannot infer target type in expression (. with-let11498 -generation)

Possibly due to a missing type hint?

I've made a minimal reproductrion of the issue here.

reagent 1.1.1
shadow-cljs 2.20.17

Edit: This warning doesn't occur when using shadow-cljs 2.20.16 or below. Seems to be a result of this

2.20.17 - 2023-01-04
[ f35fa ] add externs inference warnings for destructured bindings

grav commented

(not original poster) Seems to disappear when I downgrade Shadow-CLJS to 2.20.14

I can confirm that the linked shadow-cljs change is the cause of this. The commit changed it so that proper inference warnings are emitted for let bound names. Although no externs are necessary in this particular case the warning is still "correct".

My suggestion would be to move this entire block of code into a helper function

~(when asserting
`(when-some [^clj c# reagent.ratom/*ratom-context*]
(when (== (.-generation ~v) (.-ratomGeneration c#))
(d/error "Warning: The same with-let is being used more "
"than once in the same reactive context."))
(set! (.-generation ~v) (.-ratomGeneration c#))))

So something like

(defn check-generation! [^clj v]
  (when-some [^clj c reagent.ratom/*ratom-context*]
    (when (== (.-generation v) (.-ratomGeneration c))
      (d/error "Warning: The same with-let is being used more "
        "than once in the same reactive context."))
    (set! (.-generation v) (.-ratomGeneration c))))

and the macro just emits a function call there. This reduces the amount of code generated for every with-let and doesn't need extra hinting in the macro itself.

Actually, it should also be fine to tag the return value with ^clj.

(defn with-let-values [key]
(if-some [c *ratom-context*]
(cached-reaction (fn [] #js []) c key nil with-let-destroy)
#js []))

(defn with-let-values ^clj [key]
  (if-some [c *ratom-context*]
    (cached-reaction (fn [] #js []) c key nil with-let-destroy)
    #js []))

Maybe I can add an exception here so it doesn't warn. Normally library code doesn't warn, but since this is emitted by a macro it is treated as user code.

Would it be in any way useful if someone took @thheller's suggested change and made a pull request? I'll volunteer to do that if it helps move things along, but if that would just get in the way of @Deraen then I'll leave things be.