clojure-emacs / refactor-nrepl

nREPL middleware to support refactorings in an editor agnostic way
Eclipse Public License 1.0
256 stars 69 forks source link

have cljr-clean-ns preserve :as requires for clojurescript npm deps #383

Closed aiba closed 1 year ago

aiba commented 1 year ago

Issue

In clojurescript, :require :as with npm libraries works a little differently from jvm clojure's :require :as. This can lead to cljr-clean-ns removing requires that are needed. See also clojure-emacs/clj-refactor.el#476

Expected behavior

In both of the following cases, the require statement is needed, and I would expect cljr-clean-ns to not remove it.

(ns foo
  (:require ["@react-native-async-storage/async-storage" :as AsyncStorage])

(.getItem AsyncStorage "foo")
(ns foo
  (:require ["@react-native-async-storage/async-storage$default" :as AsyncStorage])

(.getItem AsyncStorage "foo")

Actual behavior

cljr-clean-ns removes the require in both cases.

Steps to reproduce the problem

Run cljr-clean-ns on either of the above sample code.

Environment & Version information

clj-refactor 3.5.4 (package: 20220717.2135), refactor-nrepl 3.5.3
CIDER 1.5.0-snapshot (package: 20220721.1551), nREPL 0.9.0
Clojure 1.11.1, Java 18.0.2
GNU Emacs 28.1 (build 2, aarch64-apple-darwin21.5.0, NS appkit-2113.50 Version 12.4 (Build 21F79)) of 2022-07-14
vemv commented 1 year ago

Got it. So, in cljs :as can also be considered a "refer".

Giving it a quick shot.

vemv commented 1 year ago

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)

aiba commented 1 year ago

This is working perfectly, thank you! I'm running cljr-clean-ns probably 100 times a day at work, so this really helps out. Thank you again!