Fork me on GitHub
#shadow-cljs
<
2023-02-28
>
p-himik08:02:22

Just got slightly bitten by the fact that there's no warning when a multimethod is passed a wrong amount of arguments. Is that something that's possible to add to shadow-cljs or is it on the CLJS side?

lread14:02:11

I wonder, does clj-kondo have a linter for that? Or could it?

p-himik16:02:10

Doesn't seem like it:

$ echo '(defmulti f identity) (f)' | clj-kondo --lint -
linting took 20ms, errors: 0, warnings: 0

Braden Shepherdson16:02:41

what if you add ^{:arglists ...} metadata to the defmulti?

Braden Shepherdson16:02:52

kondo might be able to spot that

p-himik17:02:03

first has arglists, clj-kondo doesn't complain still.

lread17:02:39

@U04V15CAJ, would raising a clj-kondo feature request issue for this be worthwhile?

borkdude17:02:33

I think clj-kondo could add support for this

borkdude17:02:37

issue welcome

borkdude17:02:58

arglists is never used for linting, arglists is just human-readable documentation

p-himik17:02:15

Just so the intent is not lost - while it would certainly be valuable, I would still prefer for the compiler to also complain about it. :) It's much easier to notice when your code is not reloaded with a warning right there in the UI that's being developed.

lread17:02:02

Fair enough @U2FRKM4TW! @U04V15CAJ I'll raise an issue for this over at clj-kondo.

👍 2
lread18:02:11

@U2FRKM4TW, as I write up the kondo issue, I realize my defmulti knowledge is limited. All defmethods for a defmulti would typically share the same arity, right? Would it be warning-worthy if they did not?

p-himik18:02:36

I think that's a completely different task. defmulti + calls to that function are one thing. defmethod match to its "parent" defmulti is another. But yeah, that might be something worth doing.

lread18:02:35

Thanks, so you were just originally talking about calling a defmulti with 0 args when it needs at least one, is that right?

p-himik18:02:03

With a wrong number of args, not necessarily 0-v-1, yes.

borkdude18:02:42

the dispatch function is just a regular clojure function - same rules apply

borkdude18:02:05

e.g. this should be a warning:

user=> (defmulti foo identity)
#'user/foo
user=> (defmethod foo 1 (fn [x y] y))

lread18:02:18

Ok, thanks folks, I've used defmulti but have obviously forgotten the details of how it actually works! I think I'll hold off on the kondo issue until I reboot my brain on it.

lread18:02:33

If someone else wants to raise a clj-kondo issue for this, please do go ahead.

borkdude18:02:08

what I think is happening: the dispatch function is applied to the arguments (e.g. x, or x y) and then when there's a match for a defmethod, those same arguments are fed to the defmethod function

borkdude18:02:50

user=> (defmulti foo (fn [x y] y))
#'user/foo
user=> (defmethod foo 2 [x y] {:x x :y y})
#object[clojure.lang.MultiFn 0x78830d9a "clojure.lang.MultiFn@78830d9a"]
user=> (foo 1 2)
{:x 1, :y 2}

lread18:02:18

Ok, this is probably an extra weird example usage right?: (sorry shadow-cljs folks, maybe we should move this conversation elsewhere?)

❯ clj
Clojure 1.11.1
user=> (defmulti foo :dispatch)
#'user/foo
user=> (defmethod foo :d1 [x] {:x x})
#object[clojure.lang.MultiFn 0x2f879bab "clojure.lang.MultiFn@2f879bab"]
user=> (defmethod foo :d2 [x y] {:x x :y y})
#object[clojure.lang.MultiFn 0x2f879bab "clojure.lang.MultiFn@2f879bab"]
user=> (foo {:dispatch :d1})
{:x {:dispatch :d1}}
user=> (foo {:dispatch :d2} "moar")
{:x {:dispatch :d2}, :y "moar"}
user=> (foo {:dispatch :d2})
Execution error (ArityException) at user/eval156 (REPL:1).
Wrong number of args (1) passed to: user/eval150/fn--151
user=> (foo {:dispatch :d1} "moar")
Execution error (ArityException) at user/eval158 (REPL:1).
Wrong number of args (2) passed to: user/eval140/fn--141

borkdude18:02:34

ah you're right, the defmethods can vary in arity, but have at minimum the arity of the dispatch function... I think?

borkdude18:02:19

although I think you're creating your own footgun when you're using multimethods like this

borkdude18:02:07

maybe it's #C01EF12T49W time to look for examples that exist in the wild

borkdude18:02:33

or clj-kondo also has some analysis around multimethods

borkdude18:02:35

I think it's safe to say that the amount of arguments must at least satisfy the dispatch function's arity's lower bound

borkdude18:02:39

and probably in 99% of cases the arity will be the same but we don't know this for sure

lread18:02:46

Yeah, that makes sense. I still think I'd want to understand better before raising an issue.

borkdude18:02:02

user=>  (defmulti foo :dispatch)
#'user/foo
user=> (defmethod foo :default [& xs] xs)
#object[clojure.lang.MultiFn 0x35636217 "clojure.lang.MultiFn@35636217"]
user=> (foo 1)
(1)
user=> (foo 1 2)
(1 2)
user=> (foo 1 2 3)
Execution error (ArityException) at user/eval148 (REPL:1).
Wrong number of args (3) passed to: :dispatch

lread18:02:11

I expect it would be very weird (and maybe not on purpose) for defmethods on a defmulti to have different arities?

borkdude18:02:22

:dispatch takes 2 arguments, but not 3

borkdude18:02:59

so yes, the receiver function must have arity compatibility with the dispatch function

borkdude18:02:07

since all arguments are passed to the dispatch fn

borkdude18:02:31

so we might as well lint it as if the dispatch function is called on the args directly

lread19:02:57

Should I raise a clj-kondo discussion first instead of an issue?

lread19:02:34

Will do. I'll capture what we've discussed in this thread.

thanks3 2
lread20:02:06

Here's a stab at it: https://github.com/clj-kondo/clj-kondo/discussions/2002 Any folks with insights/interest are most welcome to join in the discussion.

thheller10:02:39

not really no. well any hack is possible but it would be cleaner on the CLJS side

Gerome14:02:25

I'm getting an UnsupportedClassVersionError when using doing npx shadow-cljs watch app for the first time. I have OpenJDK 8 installed. It says com/google/javascript/jscomp/CompilerOptions has been compiled by a more recent version of the Java Runtime (class file version 55.0), this version of the Java Runtime only recognizes class file versions up to 52.0. Version 55 is Java 11. I'm on a system where the JDK is managed and I'm trying not to circumvent that because it will lead to pain. Is there anything I can do? I could point the link /usr/local/bin to something else but that is inconvenient.

Gerome15:02:42

I dropped back to shadow-cljs 2.19.6 for now. It's working now.

thheller16:02:22

@gerome.bochmann the closure compiler requires java11, so java8 is no longer supported.

Gerome06:03:19

I suspected something like this.

Maris17:02:37

I imported pikaday as (:require ["pikaday" :as pk] and then (new pk/Pikaday

Maris17:02:59

Uncaught TypeError: module$node_modules$pikaday$pikaday.Pikaday is not a constructor

Maris17:02:48

Why does it say “`not a constructor`”? They are doing the same in JS var picker *=* *new* Pikaday({ field*:* $('#datepicker')[0] }); Is it not the same thing?

thheller18:02:27

@maris.orbidans again it depends on where they got Pikaday from. consult the JS docs on what they used. either import or require

thheller18:02:25

could be as simple as ["pikaday" :as Pikaday] with just (Pikaday. ...) after

✔️ 2
thheller18:02:16

not a constructor likely just means that the pk/Pikaday doesn't even exist or is something else