re-frame-async-flow-fx icon indicating copy to clipboard operation
re-frame-async-flow-fx copied to clipboard

Please use more namespaced keywords

Open jeaye opened this issue 7 years ago • 4 comments

I particular, I think we're playing with fire when using plain ol' keywords for implementation-detail event handlers such as :forward-events. https://github.com/Day8/re-frame-async-flow-fx/blob/master/src/day8/re_frame/async_flow_fx.cljs#L42

It would also be much more respectful of the user's app and other libraries to use ::async-flow/bind, or similar, instead of the :async-flow fx currently used.

jeaye avatar Dec 21 '17 22:12 jeaye

Seems like a good idea to me. Happy for a PR, or we'll get to it eventually, though probably not before February.

danielcompton avatar Dec 21 '17 22:12 danielcompton

@jeaye why do you say "playing with fire" for using unqualified keywords for event handlers? What are the risks there? It makes it harder to adopt spec certainly, but many other libraries also use unqualified keywords?

danielcompton avatar Jun 22 '18 01:06 danielcompton

@danielcompton Yes, indeed, it's common for libraries to use unqualified keywords, but my stance is that it's no different from use and :refer :all in Clojure, since the dependencies and coupling are entirely unclear. In a system of unqualified keys, who is accessing what is quite difficult to determine. Once you introduce qualified keys, tied to require aliases, the dependencies between namespaces is explicitly described in a way that both humans and tooling (like dependency graphs) can better understand.

It's the same reason why using namespace foo; in C++ is considered poor practice, or import * in Python, or import foo.* in Java.

  1. Dependencies should be explicit
  2. Naming conflicts should be avoided by design

In this specific case, since re-frame often uses unqualified keys for internal events, if a user were to make an event with the same name, not having known it was taken, since it's an implementation detail, there's a big surprise in store. That's, in my words, playing with fire. Using qualified keys for all event names, as a practice, avoids this entirely.

I wrote a bit more about Clojure keywords here: https://blog.jeaye.com/2017/10/31/clojure-keywords/

jeaye avatar Jun 22 '18 16:06 jeaye

Ah of course, I forgot that these event handlers were part of the global re-frame handler namespace. Indeed they should be namespaced to stay out of the way of re-frame applications.

danielcompton avatar Jun 24 '18 23:06 danielcompton

I think we all agree that the proposed API changes are a good idea conceptually. However, we still think there will be too much breakage in our prod clients, not to mention those of our users. We are considering releasing a new library, though. Qualified keywords is a concern we'll certainly take into account to future-proof that project.

kimo-k avatar Nov 01 '23 22:11 kimo-k