Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update README.md #20

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update README.md #20

wants to merge 1 commit into from

Conversation

mikew1
Copy link

@mikew1 mikew1 commented Dec 9, 2024

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

@weavejester
Copy link
Owner

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.

@mikew1
Copy link
Author

mikew1 commented Dec 9, 2024

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!

@weavejester
Copy link
Owner

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.

@mikew1
Copy link
Author

mikew1 commented Dec 10, 2024

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
      ))

@weavejester
Copy link
Owner

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.

@mikew1
Copy link
Author

mikew1 commented Dec 11, 2024

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
Copy link
Author

mikew1 commented Dec 11, 2024

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).

@weavejester
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants