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

shadow-cljs emits a warning when using with-let #585

Closed
timmw opened this issue Jan 8, 2023 · 4 comments
Closed

shadow-cljs emits a warning when using with-let #585

timmw opened this issue Jan 8, 2023 · 4 comments

Comments

@timmw
Copy link

timmw commented Jan 8, 2023

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
Copy link
Contributor

grav commented Jan 8, 2023

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

@thheller
Copy link
Contributor

thheller commented Jan 9, 2023

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.

@thheller
Copy link
Contributor

thheller commented Jan 9, 2023

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.

@scarytom
Copy link

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.

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

4 participants