Closed NoahTheDuke closed 2 years ago
A comment on the PR overall - I'm not sure there's a good reason to replace a single effect with :fx
. It makes the code longer and it brings no real value. It could potentially bring more value if one wrote everything in a different way to allow easy handler composition, but it's not what this documentation is about.
So if there are at least two effects (besides :db
since its order is well defined), I would probably replace it with :fx
. But if there's just one such effect, personally I wouldn't touch it. But of course I'm not a re-frame maintainer, so that's just my own 5 c.
@p-himik Thank you for the input! I think that's a solid read of the situation. It seemed to me that the intention of the :fx
handler was to move away from "top-level" reg-event-fx
keys and to use :fx
as the primary means of performing non-db side effects. If that's not the case, then maybe the Order of Effects section should be changed so that it doesn't call using :fx
like this "best practice"?
I'll wait to make any further changes until one of the maintainers speaks up.
I appreciate the work, but I'm a little cautious about doing this. I'm still dwelling on it.
With docs, there is always a tension between:
API/reference docs should always be the former. Whereas tutorial introductions should be the latter, because they should explain concepts as simply as possible, even if they are slightly inaccurate - white lies are fine (even grey ones).
At this point, I think the docs need a "best practice" document which is aimed at the intermediate level, and THAT would be the place to insist on use of :fx
etc.
Thanks for the detailed response! Everything you mention here is good and I appreciate the delicate balance between precision and ease of understanding. I'll leave this PR open but feel free to close it as you choose.
After working more with re-frame, I think this PR is not ideal. Closing for now.
Given that the new recommended behavior is to use
:fx
inreg-event-fx
return maps, it seems prudent to use this format in all of the documentation as well. I've done my best to cover all of the usages and update the language to correctly match the implementation.I also fixed a couple mistakes around missing or extra parentheses and brackets in code blocks.