RFC: use macro

[lilactown]

One problem with the use-memo / use-effect / et al. macros is that their static analysis does not extend to custom hooks.

For instance, you cannot define a custom hook that uses the :auto-deps functionality without creating a macro and using some internal functions of helix.

The proposal here is to introduce a new macro (or two) with semantics that would allow users to define custom hooks that benefit from helix's static analysis.

;; using a built in hook
(use hooks/effect
  :once
  (foo))

;; using a custom hook
(use my-custom-hook
  :auto-deps
  #(foo))

Custom hooks could be defined using a defhook macro that could annotate the symbol with info about whether it takes deps or not.

(defhook my-custom-hook
  [^:deps deps f]
  (use hooks/effect
    deps
    (f)))

[lilactown]

One thing to consider is this would conflict with CLJS.core/use and also editors may not indent it nicely by default.

[darwin]

An alternative proposal:

1. provide with-deps macro which
   • does code analysis
   • (and maybe does some dev-time checks)
2. name helix hooks in style use-effect or use-memo and let them take deps as first arg (current impl)
3. user implementing custom hook should name it use-my-custom-hook and put :helix/hook meta flag on it, it is a normal function which can call other hooks using with-deps, it can use passed-in deps to construct derived deps as needed

Examples:

; using a built-in hook
(with-deps :auto
  (hooks/use-memo
    (foo)))

; using a custom hook is no different
(with-deps [dep1 dep2]
  (use-my-custom-hook
    (foo)))

; implementing a custom hook
(defn ^:helix/hook use-my-custom-hook [deps f]
  (let [new-deps (concat deps [dep3 dep4])]
    (with-deps new-deps
      (hooks/use-effect
        (f)))))

; what if somebody does this?
(with-deps :auto
  (println "some arbitrary code")
  (hooks/use-memo
    (foo d1))
  (println "some other code")
  (hooks/use-effect
    (bar d2 d3)))

Let's discuss the last example. TL;DR; it should still work.

with-deps macro should do deep walk of the body code and detect calls to hooks (it should look at the :helix/hook metadata).

Each use-hook call will be rewritten, like in the current implementation (including optional code analysis). Explicit deps or auto-inferred deps will be passed in as the first arg.

So the above example would rewrite roughly to something like:

(do
  (println "some arbitrary code")
  (hooks/use-memo
    #js [d1]
    (fn [] (foo d1)))
  (println "some other code")
  (hooks/use-effect
    #js [d2 d3]
    (fn [] (bar d2 d3)))

Drawbacks

If user forgets to wrap hook call in with-deps it will lead to non-sensical compilation errors. We could mitigate this with two changes:

1. with-deps would not pass deps as the first arg, but bind it as a dynamic var (defaults to :auto or :only?)
2. use-hook bodies should be normal anonymous functions, with-deps would still analyze them, but not wrap them in to (fn [] ...)

Alternatively instead:

1. dynamic var would be unbound by default, built-in hooks would have a detection that the deps are not set and would warn/error appropriatelly, this would still require use-hook bodies to be real functions.

This way even forgetting with-deps would work and behave just like :auto.
Ah, nah. The default must be :only because without with-deps no code analysis could run.

Further ideas

1. allowing with-deps to be nested?
2. we could implement advanced code analysis in defnc, which would walk whole body and warn/error on hook usages behind conditional code

[lilactown]

I like the idea of moving hooks deps detection to the defnc macro and a defhook macro; the defhook macro can do the work of annotating the custom hook var with necessary metadata to determine how to analyze the hook, and analyzing hooks within the custom hook body.

I have been working on code analysis inside defnc body, detecting hooks usages in conditionals and loops. It's still very WIP, but can be activated in master by using a feature flag.

I think I am less bullish on the auto-deps functionality than I was before. I am more interested in detecting when deps are not filled in fully, and warning (with the ability to ignore). I am fine with not extending :auto-deps for all hooks as long as we can extend the analysis of deps to all hooks.