Skip to content

Commit

Permalink
Simplify implementation of Middleware Compose
Browse files Browse the repository at this point in the history
The implementation of middleware compose is duplicating behavior already
in compose.

`(apply comp [])` returns identity.
  • Loading branch information
estsauver committed Jul 11, 2015
1 parent e991717 commit 9326a41
Showing 1 changed file with 3 additions and 6 deletions.
9 changes: 3 additions & 6 deletions src/re_frame/handlers.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,9 @@

(cond
(fn? v) v ;; assumed to be existing middleware
(vector? v) (let [v (remove nil? (flatten v))
_ (report-middleware-factories v)] ;; damn error detection! always messes up the code
(cond
(empty? v) identity ;; no-op middleware
(= 1 (count v)) (first v) ;; only one middleware, no composing needed
:else (apply comp v)))
(seq? v) (let [v (remove nil? (flatten v))]
(report-middleware-factories v)
(apply comp v))
:else (warn "re-frame: comp-middleware expects a vector, got: " v)))


Expand Down

2 comments on commit 9326a41

@darwin
Copy link
Owner

@darwin darwin commented on 9326a41 Aug 17, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This broke todomvc example. For example this call:
https://github.com/Day8/re-frame/blob/b566c08f40d0485941e8e06f7678e8064563cde3/examples/todomvc/src/todomvc/handlers.cljs#L53

Vector of middlewares is passed in which gets into comp-middleware as "v" param. (seq? v) returns false and :else branch is executed.

@darwin
Copy link
Owner

@darwin darwin commented on 9326a41 Aug 17, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw. fixed it in my branch here:
329a57f

Please sign in to comment.