oakmac / standard-clojure-style-js

Standard Clojure Style in JavaScript
ISC License
78 stars 1 forks source link

`:use` is removed from ns declarations #152

Open pbaille opened 2 weeks ago

pbaille commented 2 weeks ago

I've been surprised by this behavior:

echo '(ns my.company.core (:use my-thing))' | standard-clj fix - => (ns my.company.core)

It removes the :use statement. It seems a bit radical in my opinion, of course using :use is rarely justified but should it at least be replaced by a :refer :all ?

A part from that I really enjoy this formatter for now, so thank you :)

oakmac commented 2 weeks ago

I have been waiting for someone to open an issue about this :wink: I have "support use" as a TODO item on Issue #5 and I never got around to finishing it.

I am not sure of the best behavior for the formatter here. I consider the current behavior to be wrong / a bug. Two thoughts:

I think the second option is the better user experience, but I am not sure how challenging it will be to code up. The first option is not great, but better than the current behavior.

This comment on Issue #142 is relevant. It may seem odd that a formatter recommends the user to "please refactor", but presumably if the user is using this tool then they desire the end state for their code to be something produced by the tool, and I do not have plans to add use into the pretty-printed ns form.


A part from that I really enjoy this formatter for now, so thank you :)

Thank you :grin: :tada:

NoahTheDuke commented 2 weeks ago

I do not have plans to add use into the pretty-printed ns form.

Why not? There is a lot of code that uses :use and some people still prefer it to :require (especially with clojure.test).

oakmac commented 2 weeks ago

Why not?

My main reasoning is that use is not recommended by how to ns and my general understanding is that require is preferred over use for basically all use cases. Is there a situation in which use is preferable to require in the ns form?

NoahTheDuke commented 2 weeks ago

It's not about preference, it's about supporting existing working code. (For example, Clojure core is filled with :use, and that won't ever change.)

It's okay if Standard is only for the subset of Clojure you like, but I think that should be made more clear.

oakmac commented 1 week ago

It's not about preference, it's about supporting existing working code. (For example, Clojure core is filled with :use, and that won't ever change.)

Some thoughts:

The current behavior is definitely a bug and needs to be fixed with either of the two options I outlined earlier. Option 1 is probably the easiest to code up right now, and Option 2 is a superior user experience.

My understanding is that :require is always preferred over :use inside the ns form. I have no expectation that Clojure core will stop supporting :use, but that does not mean it is recommended for newer code. The authors of ClojureScript decided not to include :use in the language, for example. To the degree that Standard Clojure Style can make a codebase better, that is the right direction to move towards.

Standard JS is a good reference point here. == and eval() are valid parts of the JavaScript language that will likely always be supported, but that does not mean they are recommended to be used. It is ok to say "This feature of a language was maybe not the best idea. As a team we are ok with not using it."