clj-kondo 2023-02-07 | Slack Archive

Braden Shepherdson16:02:06

Metabase uses potemkin/import-vars in lots of places. I'm porting a bunch of things to CLJC, and Potemkin doesn't support CLJS at all. I've written this macro as an impoverished potemkin/import-vars for functions only that works cross-platform:

(defn- redef [target sym]
  (let [defn-name (or sym (symbol (name target)))]
    `(def ~defn-name (fn [& args#] (apply ~target args#)))))

(defmacro import-fns
  "Imports defns from other namespaces.
  This uses [[import-fn]] to create pass-through local functions that reload nicely.
  `(import-fns [ns1 f1 f2 f3] [ns2 f4 f5])` creates `f1` that calls `ns1/f1`, `f2` that calls `ns1/f2`, etc.
  If you need to rename a function, instead of just the function name, pass `[original new-name]`."
  [& spaces]
  `(do
     ~@(for [[target-ns & fns] spaces
             f                 fns
             :let [target-sym (if (vector? f) (first f)  f)
                   new-sym    (if (vector? f) (second f) f)
                   target     (symbol (name target-ns) (name target-sym))]]
         (redef target new-sym))))

and it works in the code proper, in both CLJ and CLJS. but clj-kondo doesn't grok this, of course. I wrote a :macroexpand helper:

(defmacro import-fns
  "Kondo macro replacement for [[metabase.shared.util.namespaces/import-fns]]."
  [& pairs]
  `(do ~@(for [[from & syms] pairs
               sym           syms
               :let [args (gensym)]]
           `(def ~sym "docstring" (fn [& ~args] (apply ~(symbol (name from) (name sym)) ~args))))))

which doesn't seem to work - I still get warnings about unresolved symbols in the macro call, the :required namespace is flagged as unused, and calls of the imported symbols farther down in the file are also unresolved. I know kondo has special magic for Potemkin's import-vars - is there hope for me to handle this at the kondo config level, or do I need equivalent magic built in? (alternatively, is there a CLJC-friendly alternative to potemkin/import-vars?)

lread17:02:12

I'm not opposed to the idea of importing vars at load time, but it is not popular in the Clojure community. My take is that it is more trouble than it is worth. I found myself dealing with too many mysteries with tooling related to its usage. That said, when I took over maintaining rewrite-clj, it used an inlined version of import-vars. When I merged in rewrite-cljs, I created an inlined cljs version of import-vars. Ultimately, I abandoned this load-time approach to importing vars and went with code generation instead. No more import-vars mysteries for me!

Braden Shepherdson17:02:47

I found and read that issue thread, actually.

Braden Shepherdson17:02:07

I've managed to hack around the issue by (a) expanding my macro above to hand off to potemkin/import-vars in CLJ context and the above code in CLJS context, and (b) doing :lint-as {metabase.shared.util.namespaces/import-fns potemkin/import-vars} in kondo.

Braden Shepherdson17:02:27

that's more than a little bit of a hack, but it gets me access to the potemkin magic in kondo, and everything Just Works.

Braden Shepherdson17:02:21

(it also gives the nicer REPL reload behavior of Kondo in CLJ mode. it's not relevant in CLJS REPLs given the separate compilation step)

borkdude21:02:20

Nice, so :lint-as worked for you if I understand correctly

Braden Shepherdson22:02:35

yes, it seems that it worked nicely, including the special case magic for potemkin.

👍 2

Matthew Davidson (kingmob)08:04:09

> (alternatively, is there a CLJC-friendly alternative to potemkin/import-vars?) Feel free to submit a PR!

Matthew Davidson (kingmob)08:04:18

^^^ @UCY0GT0QM

Braden Shepherdson13:04:57

not exactly. p/import-vars does quite a bit of wizardry with vars that does not play very nice in CLJS (where vars aren't real at runtime) the closest CLJC-friendly approximation I'm aware of is the one we wrote in metabase.shared.util.namespaces https://github.com/metabase/metabase/blob/master/shared/src/metabase/shared/util/namespaces.clj it works, but not quite as nicely as Potemkin in terms of handling metadata and go-to-definition etc.

lread13:04:26

@U10EC98F5 I did write a cljc compatible version of import-vars within rewrite-cljc (now archived and replaced by rewrite-clj v1). Because import-vars has caused me so many headaches and un-fun mysteries, I am hesitant to spread this work to the wild and am not excited about supporting such a thing.

Matthew Davidson (kingmob)01:04:09

I'll defer to you two, then.

Braden Shepherdson02:05:14

I mean we're using it without issues so far at Metabase, but it doesn't really feel like it's worth publishing separately. unfortunately it's different enough from the original Potemkin implementation that it's hard to contribute it there.

2023-02-07

Channels