ring-clojure / ring

Clojure HTTP server abstraction
MIT License
3.75k stars 519 forks source link

documentation for using wrap-reload #104

Open trevor opened 10 years ago

trevor commented 10 years ago

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

It appears wrap-reload needs to be passed a handler as normal, but with the caveat that its referenced through a var (see https://github.com/mmcgrana/ring/issues/72).

Something along the lines of:

(def app
  (-> (compojure.core/routes my-routes)
      (compojure.handler/api)))

(def app-with-reload
  (ring.middleware.reload/wrap-reload #'app))

(defn -main []
  (ring.adapter.jetty/run-jetty #'app-with-reload {:port 8081 :join? false})
  (ring.adapter.jetty/run-jetty #'app-with-reload {:port 8082 :join? false}))

If instead wrap-reload is treated as a normal middleware—placed in the chain for app—it seems to work but the updated behavior is actually delayed by one request.

Raising this issue to hopefully get some documentation in wrap-reload that makes note that it needs the var reference; also it may be helpful for it to throw an exception if it's not passed a var to prevent accidental use as typical middleware.

weavejester commented 10 years ago

The wrap-reload middleware reloads namespaces. It doesn't require a var, but you do need to be careful of the order of evaluation.

In the example you give, if app is passed directly to wrap-reload, then the middleware will retain the old handler. Only when the next request comes in, with the app-with-reload var pointing to a new value, will the updated handler be used.

This isn't anything specific to wrap-reload, it's just a natural consequence of the structure of the namespace and how Clojure handles reloads.

I think this would be better placed in a wiki page on namespace reloading, as it's a subject that will take at least several paragraphs to cover. The docstring of wrap-reload is accurate, so long as you understand how Clojure reloads namespaces.

trevor commented 10 years ago

Thanks for taking the time to followup, all of what you say makes sense. A wiki page with more extensive explanation (and accompanying docstring note to it) would be a great help I think.

I agree that the docstring is accurate, my concern is that as a middleware it runs a little contrast to the usual pattern(s) of tying middleware together, which would almost certainly confuse someone using it for the first time without a little heads-up.

micrub commented 9 years ago

I also have remark about wrap-reload. Ring throws NullPointer exception if routes are misspelled for example, which is kind of confusing for new comers.

GetContented commented 9 years ago

While developer friendliness is definitely an aspect to consider, so is the efficiency and size of the created production artefact (ie the compiled program that will end up in production use).

If the project is going to go down the path of baking documentation and bug-catching helpful pointers into the codebase and its exceptions, it's definitely advisable to make production compilation options not include this cruft that is only useful for developers. Creating such a program is a much more complicated thing to do than, say, just recommending newcomers read the documentation, recommending that they know how to debug their code properly and ensuring that there are adequate FAQ / troubleshooting articles in place. ( @micrub you could possibly spearhead this effort, as you care enough to comment on it ).

weavejester commented 9 years ago

Ring throws NullPointer exception if routes are misspelled for example

I don't understand what you mean by this, @micrub.