cljr-clean-ns doesn't support shadow-cljs's npm import syntax

[lucywang000]

Expected behavior

Given such a file:

(ns foo
  (:require ["react" :as react]
            ["@material-ui/core/Button" :default Button]))

(println react)
(println Button)

After M-x cljr-clean-ns both requires should be preserved.

Actual behavior

Both imports are removed, even though they are used in this ns.

Versions

• MacOS 10.15
• Java8
• Emacs 26.3
• refactor-nrepl 2.5.0
• clj-refactor.el 20200405.1419

Is this not supported, or am i missing something that could make it work?

[expez]

Is this not supported

It's not supported because it isn't valid clojure code. shadow-cljs didn't exist when the original code was written and nobody has added support for these kinds of libspecs (yet!).

I searched the clojure(script) docs again, briefly, and I can't find anything stating that this is OK. I guess that means shadow-cljs could break at any moment because they're relying on unspecified behavior in the compiler?

Anyway, happy to take a PR over in refactor-nrepl if you want to get involved, but I'm not going to work on this myself.

[thheller]

FWIW the string :require syntax is not shadow-cljs specific and started in the official CLJS codebase. shadow-cljs was just the first tool to make extensive use of it. I cannot find the relevant issue but it was added shortly before this post from 2017 was published.

[aiba]

Just chiming in to say I would really appreciate it if cljr-clean-ns would keep these types of imports. Also we could rename this issue to "cljr-clean-ns doesn't support npm import syntax", since as @thheller pointed out, this is not specific to shadow-cljs.

[vemv]

@aiba : hi there!

You created some related commits in refactor-nrepl which remain there since. What's been your experience since?

What works and what doesn't?

[vemv]

Alright so here's the deal!

• String requires indeed have worked for a long time.
• Unused string requires are removed like any other require
  • You can create exceptions or disable the feature; please refer to the doc
• :default isn't supported in face of the following:

If you find a new, specific issue please feel free to open one (from scratch).

Cheers - V

[aiba]

Thank you for looking into this!

Ignoring the shadow-cljs-specific :default syntax, I think there remains an issue in clojurescript where you require a npm dep :as foo and then use foo for its value (which is a weird cljs/npm thing but required in many cases). I'll create a new issue.

[vemv]

For anyone else watching: said issue is now fixed in refactor-nrepl 3.5.4 (released within a few minutes) and clj-refactor 3.5.5 (available in MELPA within a couple hours)