ring-refresh icon indicating copy to clipboard operation
ring-refresh copied to clipboard

Update README.md

Open mikew1 opened this issue 1 year ago • 8 comments

Don't know if this helps at all. Really just a reflection of my current understanding. Thanks :)

mikew1 avatar Dec 09 '24 21:12 mikew1

I'd be extremely surprised if there was any incompatibility between Reitit and wrap-refresh, as long as wrap-refresh is applied as outer middleware, and not per-route.

weavejester avatar Dec 09 '24 22:12 weavejester

I've been doing it like this: (adapter/run-jetty (wrap-refresh (wrap-reload #'app)) {:port 3000}))

(And not using 'reitit middleware' - using entirely ring middleware after converting the reitit router into a ring handler).

Thinking out loud, I guess I could alternatively put wrap-refresh inside app at the foot of its threading macro, so it technically would get reloaded if it were to change (though it won't). If I understand correctly!

mikew1 avatar Dec 09 '24 23:12 mikew1

That seems fine. There shouldn't be any interaction point between Reitit and wrap-refresh, as from wrap-refresh's point of view Reitit is a black box, and from Reitit's point of view wrap-refresh may as well not exist.

weavejester avatar Dec 09 '24 23:12 weavejester

Hmm I'm a bit stumped on this currently. But just to check I'm not doing anything silly with the middleware stack before it gets to wrap-refresh?

(def app
  (-> routes
      rr/router                     ; reitit router is a great big object
   
      ;rr/ring-handler              ; but for ring it must be a function
      ((fn [router] (rr/ring-handler router (constantly (response "default route")))))
   
      ; ring middleware in traditional style
      ;(wrap-authentication buddy-backend)
      ;(wrap-authorization buddy-backend)

      (wrap-session {:store (cookie-store {:key (string-to-key "thisis16charslng")}) :cookie-attrs {}})

      wrap-keyword-params    ; make params keywords
      wrap-nested-params     ; make params nested
      wrap-params            ; parse :query-params, :form-params, & merge to :params

      wrap-cookies
      wrap-refresh           ; add js to response, & a route that it calls back to.
                             ; but, periodic ~30s page load pause (consistently) unless wrap-refresh is commented out
      ))

mikew1 avatar Dec 10 '24 10:12 mikew1

If the wrap-refresh is in a namespace that is expected to be reloaded, it may be creating multiple watchers. Do you find the slowdown happens immediately, or after a number of reloads?

Another possibility is that your src or resources directory may be very large, perhaps due to generated files, and each time it's loaded it's iterating through a large file tree.

weavejester avatar Dec 10 '24 18:12 weavejester

The slowdown happens after a number of reloads. (And when I say 'reload' - here I mean browser refresh (i.e. page view). This is on a fresh restart of the JVM and then not changing anything in src dirs or anywhere on the filesystem - the long pauses happen in this case).

However, generally, is it important to avoid having the middleware stack (including in this case wrap-refresh) in a namespace that may be reloaded (i.e. with wrap-reload)? (though in this case as mentioned the pause happens even when I'm not changing any source files, so i'd have thought no reload would be being triggered).

I can sense that conceptually that might be an issue; but without full understanding of how it's working I don't know. Currently I'm using a single file & namespace for the whole application.

Just now I moved the middleware stack and the server invocation to a separate file & namespace, to find out if that eliminated the pauses, but it didn't; they slightly increased.

This isn't a critical issue at this immediate point, so what I may do next on this is add the code from licht1stein's fork manually, find out if that works, and make sure I understand it, then proceed from there to try to diagnose further.

Separately, I wondered if there's a unix process I can check for to check whether multiple watchers are running? Or if that's all coming from the same jvm, it seems perhaps not.

mikew1 avatar Dec 11 '24 10:12 mikew1

I've switched to licht1stein's fork, since that's known to be working with Reitit, and I get no 30-40 second pause as before. (Just had to make sure I was injecting the right js file!) (as I pasted the code manually into my project in order to explore it).

mikew1 avatar Dec 11 '24 14:12 mikew1

If it happens after a number of reloads, that implies that wrap-refresh is leaking threads or some or resource.

Fixing this might be moot, as @licht1stein has an in-progress PR based on their fork that may be ready to merge at some point.

weavejester avatar Dec 11 '24 19:12 weavejester