macchiato-framework / macchiato-core

Ring style HTTP server abstraction for Node.js
MIT License
377 stars 35 forks source link

Using express/connect middleware #13

Closed facundoolano closed 7 years ago

facundoolano commented 7 years ago

I've been wondering about the possibility of plugging in Express/Connect middleware into a Macchiato app, is this something you have considered?

I understand the priority of the framework is to use Ring style middleware, but as a library user I'd like to be able to easily leverage the existent npm modules rather than having to code them myself in the cases where there isn't a Macchiato/Ring library that already does the job.

After reviewing the macchiato code I imagine it doesn't make much sense to try to mix Node and Clojure middleware once the request has been "translated" to cljs, but perhaps there could be a way to customise the function that's passed to the node http server (like allowing to wrap it or passing a list of node middlewares to apply to it).

yogthos commented 7 years ago

I agree that mixing Express/Connect middleware with Macchiato handlers is not going to work well. Conversely, I also think it would be nice to allow leveraging the existing ecosystem, especially as Macchiato itself is quite young.

Currently, the request map contains keys called :node/request and :node/response. These contain the original Node request/response objects. A middleware function could use these to hook in Node specific middleware.

facundoolano commented 7 years ago

Yes, I saw those keys. At first I thought they weren't enough because the Macchiato response function wraps around the initial state of the :node/response. But on second thought it's node, not Clojure, so the node middleware must just mutate that same response object, therefore changes to :node/response should affect the response Macchiato wraps at the beggining.

Anyway, I'm going to experiment a bit with some common node middlewares and see what I find out.

yogthos commented 7 years ago

Let me know how it goes, maybe we could add some default middleware helpers to make using Node middleware easier.

facundoolano commented 7 years ago

Ok, so I made a couple of simple tests and this is what I came up with so far:

(defn populate-req-map
  "Translate (js->clj) each property from the node request into the req map."
  [req mappings]
  (let [node-req (:node/request req)]
    (reduce (fn [req [key prop]]
              (let [value (js->clj (aget node-req prop))]
                (assoc req key value)))
            req mappings)))

(defn wrap-node-middleware
  "Executes the given Node.js middleware function with the original
  :node/request and :node/response objects before calling the handler.
  The :req-map maps properties from the resulting :node/request object to keys
  in the request map passed to the handler (js->clj is applied to the values)."
  [handler node-mw & {:keys [req-map]}]
  (fn [req res raise]
    (let [next (fn [err]
                 (if err
                   (raise err)
                   (handler (populate-req-map req req-map) res raise)))]
      (node-mw (:node/request req) (:node/response req) next))))

Using that function I added some simple express middleware to my app:

(def body-parser (node/require "body-parser"))
(def response-time (node/require "response-time"))
(def morgan (node/require "morgan"))
(def favicon (node/require "serve-favicon"))

(defn app []
  (http/start
   {:handler    (-> router
                    (wrap-node-middleware (.text body-parser)
                                          :req-map {:body "body" :text "body"})
                    (wrap-node-middleware (.json body-parser)
                                          :req-map {:body "body" :json "body"})
                    (wrap-defaults)
                    (wrap-node-middleware (favicon "public/clojure.ico"))
                    (wrap-node-middleware (morgan "combined"))
                    (wrap-node-middleware (response-time)))}))

This seems to work well at least for these simple cases: it adds a response time header, prints an apache-like log to stdout, serves a favicon and parses json and text request bodies. I added that :req-map option to easily feed back stuff from the updated :node/request to the req map, which seems like a common enough scenario.

Here's my app using the code above. Let me know your thoughts, I can make this into a PR if you find it useful.

yogthos commented 7 years ago

This look great, and it's great to see that mixing Node middleware can be relatively painless. I think this would be quite useful to have, and a pr would be great. We should add your example to the examples repo as well for reference. :)

facundoolano commented 7 years ago

I'm closing this since the issue is addressed by the merged PR. I'll write a reduced version of my test app for the examples repo once you publish a new version of macchiato-core.

yogthos commented 7 years ago

👍