Autoreload on file change or on evalutation

[elmehalawi]

Is it possible to autoreload or auto-reset when a file changes, or when form is evaluated in a namespace?

[yogthos]

The closest would be to call (user/refresh) from the REPL.

[elmehalawi]

Does that need to be called on each change?

[yogthos]

Yeah, you'd have to call it explicitly when you want the REPL state refreshed.

[elmehalawi]

Is it possible to have this called on file change, or when something is evaluated in the repl?

[yogthos]

Generally, that wouldn't be the desired behavior. You shouldn't have to reload the entire REPL state any time you evaluate something in the REPL. Typically, it's best to do a full reload explicitly with a command instead of having it run automatically as it's a slower operation and it can break the REPL state if files aren't in a good state.

[elmehalawi]

That was my first thought initially too. But in my project if I evaluate a function that returns some hiccup, that change isn't reflected when I reload the browser page. Does that mean my project is misconfigured, or is this the expected behavior?

[markokocic]

This is more of a reitit problem than kit. Once routes are defined, the handler is bound to the route. Even if you recompile the handler function, reitit will still use the old one.

One workaround is that you define your routes to use functions.

For example, instead of the following:

(defn ui-routes [_opts]
  [["/" {:get home}]
   ["/clicked" {:post clicked}]])

you wrap your handlers with function in your route definitions

(defn ui-routes [_opts]
  [["/" {:get #(home %)}]
   ["/clicked" {:post #(clicked %)}]])

This is not elegant, but it works.

The right fix would be to implement a solution from reitit directly in kit. Basically, having one non-efficient but reloadable router for development, and one non-reloadable but efficient for production.

[yogthos]

I think that would be a useful change to make. @nikolap thoughts?

[nikolap]

Great find and makes sense to me, thanks for the research and recommendation.

Would welcome a PR. Else I can look at it in late-July.

[nikolap]

Haven't forgotten this issue! Only just found the time to look at it :)

[yogthos]

Do we still need to figure out a fix here?

[elmehalawi]

I haven't tested master, but for my day to day workflow I still need to reset on each and every change.

[nackjicholson]

@yogthos @nikolap What do y'all think the level of effort might be to get this working for kit? I'm almost certain luminus autoreloaded after repl evals, and I would find this really valuable for fast feedback and pace of development. I don't know if it's something I could contribute, and that's why I ask the difficulty level.

[yogthos]

I think it should be relatively straightforward. We could add a profile based flag in system.edn to let us know whether we're initializing the system in dev or prod mode. Then the multimethod for the router could use the flag to decide whether to initialize the router as a function or not. These should be the only changes needed.

[gerdint]

Doesn't Kit by virtue of being based on Integrant adhere to the reloaded workflow school? I.e. you explicitly synchronize the whole system with the contents of source files on disk (using reset) instead of relying on traditional Lisp piecemeal reevaluation? Apart from the reitit handlers other functions referenced from the system map (such as query-fn, and I have several other components whose instances are funs) will also point to previous references even if you reevaluate them no?

I wasn't aware of this when I first started using Kit (and I haven't used Component) so I was a bit confused when I first started using Kit, but now I think that just binding a key to (reset) works quite well (though it admittedly takes a few of seconds on my system to suspend and resume)

[nackjicholson]

@gerdint I will take a moment to read the blog you provided in full. I am very open to adjusting my workflow expectations. If I have to call (reset) from the REPL, and there are good advantages to that, then no problem!

@yogthos If there are benefits to how things are, perhaps it's only worth a small documentation update to educate users like me on the virtues of this workflow. I think if I hadn't expected the workflow to be like other lisps and Luminus, I would've moved on without a second thought.

[yogthos]

@gerdint @nackjicholson yup, reloading the routes won't help with other components, so you'd still need to do (reset) whenever those change. Having the router reload automatically could potentially add confusion to the workflow since sometimes things will work as expected and sometimes not.

The main advantage of the reloaded workflow is that it avoids having stale state. One problem that can often happen with the regular incremental loading workflow is that the REPL state gets out of sync with the source. For example, you add a function and eval it, then you delete it from the file. The function is still in the REPL runtime and stuff using it keeps working. So you don't realize you broke anything until you restart the REPL. Reloaded workflow avoids this problem by nuking the REPL state and rebuilding it when (reset) is called.

My approach has been to do incremental work in the REPL for the most part, and then run (reset) when I want to test all the changes up to the API level. I find I also tend to add helpers like the following in the user ns:

(defn api-ctx []
  {:query-fn (:db.sql/query-fn state/system)})

So, then when I'm working on controllers from the REPL, I just pass (user/api-ctx) to them to simulate calling them from the router. For the most part I find my workflow hasn't changed from Luminus all that much. The only time reset is needed is when changes to one of the components managed by the integrant is needed. Most of the time that's when you change routes, or db queries.

Adding some docs on the workflow is definitely a good idea. I'll add an issue for that.

[markokocic]

Reloading components and redefining a route handler are two different things. I agree that (reset) is a good idea when defining a new or changing an existing component or it's config. On the other hand, redefining a handler function of an existing route in REPL should just work in development, since it's just a function, and not a component with it's own state.

[yogthos]

I think the added convenience of being able to see changes reflected as you update the handlers is likely worth it. My vote would be to add the convenience for dev along with a bit of documentation on the REPL workflow which I've made an issue for already.

[gerdint]

On this sort of same note I think it would be nice if HugSQL query defintition files (the .sql ones) also were not cached in dev (i.e. a simple save of the file should have Kit use the new queries without having to reset the component). This is in-line with the change we did for Selmer templates the other week I think.

[yogthos]

That's a good idea. Perhaps we could update kit-sql-conman to use a similar trick that Reitit uses where we can evaluate a function on each call in dev mode and load queries once for prod.

and added an issue for this #128

[zengxinhui]

https://github.com/ring-clojure/ring/blob/master/ring-devel/src/ring/middleware/reload.clj
https://ring-clojure.github.io/ring/ring.middleware.reload.html

This works in some use cases.

[yogthos]

The main caveat with reload middlware is that it won't reload the components such as routes or sql queries. If there is interest, it might be easy enough to make a Kit specific version of this middleware that would also trigger Integrant system reload when it detects file changes. That way the system would always be in a clean state.