re-frame-async-flow-fx
re-frame-async-flow-fx copied to clipboard
Please use more namespaced keywords
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.
Seems like a good idea to me. Happy for a PR, or we'll get to it eventually, though probably not before February.
@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 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.
- Dependencies should be explicit
- 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/
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.
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.