From a48f55ff79fde1a43bb93c347d4525144e191f56 Mon Sep 17 00:00:00 2001 From: mike-thompson-day8 Date: Thu, 5 Mar 2015 14:43:11 +1100 Subject: [PATCH 1/7] With pure much more firmly embedded as mandatory middleware, remove a couple of checks --- src/re_frame/middleware.cljs | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/src/re_frame/middleware.cljs b/src/re_frame/middleware.cljs index a9f8960..89d7426 100644 --- a/src/re_frame/middleware.cljs +++ b/src/re_frame/middleware.cljs @@ -18,17 +18,18 @@ [handler] (fn pure-handler [app-db event-vec] - (assert (satisfies? IReactiveAtom app-db) - (str "re-frame: pure not given a Ratom." - (if (map? app-db) - " Looks like \"pure\" is in the middleware pipeline twice." - (str " Got: " app-db)))) - (let [orig-db @app-db - new-db (handler orig-db event-vec)] - (if (nil? new-db) - (warn "re-frame: your pure handler returned nil. It should return the new db.") - (if-not (identical? orig-db new-db) - (reset! app-db new-db)))))) + (if (not (satisfies? IReactiveAtom app-db)) + (do + (if (map? app-db) + (warn "re-frame: Looks like \"pure\" is in the middleware pipeline twice. Ignoring.") + (warn "re-frame: \"pure\" middleware not given a Ratom. Got: " app-db)) + handler) ;; turn this into a noop handler + (let [orig-db @app-db + new-db (handler orig-db event-vec)] + (if (nil? new-db) + (warn "re-frame: your pure handler returned nil. It should return the new db state.") + (if-not (identical? orig-db new-db) + (reset! app-db new-db))))))) (defn debug @@ -38,13 +39,11 @@ [handler] (fn debug-handler [db v] - (if (satisfies? IReactiveAtom db) - (str "re-frame: \"debug\" middleware used without prior \"pure\".")) (group "re-frame event: " v) (let [new-db (handler db v) diff (data/diff db new-db)] (log "only before: " (first diff)) - (log " only after: " (second diff)) + (log "only after : " (second diff)) (groupEnd) new-db))) @@ -79,14 +78,12 @@ If a get-in of the path results in a nil, then \"default-fn\" will be called to supply a value. XXX very like update-in. Should the name be more indicative of that closeness? " ([p] - (path p hash-map)) + (path p #(nil))) ([p default-fn] (fn middleware [handler] (fn path-handler [db v] - (if (satisfies? IReactiveAtom db) - (str "re-frame: \"path\" used in middleware, without prior \"pure\".")) (if-not (vector? p) (warn "re-frame: \"path\" expected a vector, got: " p)) (let [val (get-in db p) @@ -126,7 +123,8 @@ position presumably for side effects. \"f\" is given the value of \"db\". It's return value is ignored. Examples: \"f\" can run schema validation. Or write current state to localstorage. etc. - In effect, \"f\" is meant to sideeffect. It gets no chance to change db. See \"derive\" (if you need that.)" + In effect, \"f\" is meant to sideeffect. It gets no chance to change db. See \"enrich\" + (if you need that.)" [f] (fn middleware [handler] From f5e2d484417ec1037ceff1bf177f537aa9ae893a Mon Sep 17 00:00:00 2001 From: mike-thompson-day8 Date: Fri, 6 Mar 2015 10:47:28 +1100 Subject: [PATCH 2/7] Add a Changes doc --- CHANGES.md | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 CHANGES.md diff --git a/CHANGES.md b/CHANGES.md new file mode 100644 index 0000000..cdf28c0 --- /dev/null +++ b/CHANGES.md @@ -0,0 +1,70 @@ + + +## Planned for v0.3.0 + + - automatically wrap subscriptions in a `reaction` (removing the need for over + 10 keystrokes per handler!!). Just kidding there are better reasons than that. + - produce a more fully featured todomvc (beyond the standard one), todomvc-with-extras + - use `enrich` to handle todo duplication + - show testing + - show debug + - begin to use goog.Logger ?? How to let client apps know about exceptions, etc ?? + + +## v0.2.0 (2015-03-06) + +Being blocked by: + +In todomvc, to be done tomorrow ahead of release: + - fix bug in todomvc which means the footer disappears and doesn't come back. + - add localstorage + - add history, back button etc. + +### Breaking + +Renames: + - `register-pure-handler` renamed to `register-handler` (and existing low level `register-handler` becomes `register-handler-base` but is not a part of the API). + - remove `apply-event` middleware and replace with similar `trim-v` + - rename `register-subs` to `register-sub` (avoid confusion over possible plurals) + - rename `set-max-undos` to `set-max-undos!` + +Changes: + - `undoable` middleware is now a factory. Where before you used this `undoable`, + you must now use this `(undoable "some explanation")`. See further below. + +### Headline + + - example `todomvc` available in examples folder. + - Wiki documentation is now more substantial. + - introduce some new handler middleware: `debug`, `enrich` and `after` + +### Other: + - exceptions in a go loop are a special type of hell. Improve the reporting of exceptions happening in event handlers. + - allow Middleware to be registered as a vector. data > functions > macros + - fix two bugs in undo implementation + - name licence file correctly, thanks to @smith + - fix typo in readme, thanks to @btheado + - Readme now admits to 200 lines of code, not 100. + - dispatch now explicitly returns nil + - travis integration (not that we have any tests currently) + + + +### Details On Undo Changes + +We've made changes so that the undo/redo feature is more powerful. Associated +with each undo state is an explanation which can be presented to the user to +inform them as to the actions they will be undoing or redoing. + +Previously `undoable` was simply middleware, but it is now a middleware factory. + +Essentially that means you can't use it "plain" anymore, you must call it to +get middleware (put it in `()`) and, when you do, supply a parameter which is +an explanation for the mutation. "Set spam flag to yes", "add todo", etc. + +When the time comes to present undos to the user, you then have an explanation +associated with each one. + +The `explanation` provided to undoable must be either a `string` (static +explanation) or a function `(db event) -> string` (allowing you to customize +the undo message based on details of the event. "Added todo called blah blah blah"). From 0ebb46cb9124f8d45c93e742e9b37d5d23f9cd6f Mon Sep 17 00:00:00 2001 From: mike-thompson-day8 Date: Fri, 6 Mar 2015 10:48:00 +1100 Subject: [PATCH 3/7] undoable is now a middleware factory --- src/re_frame/middleware.cljs | 43 +++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/src/re_frame/middleware.cljs b/src/re_frame/middleware.cljs index 89d7426..342dffb 100644 --- a/src/re_frame/middleware.cljs +++ b/src/re_frame/middleware.cljs @@ -11,10 +11,14 @@ (defn pure "Acts as an adaptor, allowing handlers to be writen as pure functions. - The re-frame router will pass in an atom as the first parameter. This middleware - adapts that to the value within the atom. - If you strip away the error/efficiency checks, this middleware is just: - (reset! app-db (handler @app-db event-vec))" + The re-frame router will pass in an atom as the first parameter. This + middleware adapts that atom to be the value within the atom. + If you strip away the error/efficiency checks, this middleware is just doing: + (reset! app-db (handler @app-db event-vec)) + You don't have to use this middleare directly. Is supplied by default + when you use register-handler. + The only way to by-pass use of pure is to use the low level registration function + re-frame.handlers/register-handler-base" [handler] (fn pure-handler [app-db event-vec] @@ -48,14 +52,6 @@ new-db))) -(defn undoable - "Middleware which stores an undo checkpoint." - [handler] - (fn undoable-handler - [app-db event-vec] - (store-now!) - (handler app-db event-vec))) - (defn trim-v "Middleware which removes the first element of v which allows you to write @@ -72,7 +68,8 @@ (defn path - "Supplies a sub-tree of `db` to the handler. A narrowed view. + "A middleware factory which supplies a sub-tree of `db` to the handler. + Supplies a narrowed view. Assumes \"pure\" is in the middleware pipeline prior. Grafts the result back into db. If a get-in of the path results in a nil, then \"default-fn\" will be called to supply a value. @@ -91,6 +88,26 @@ (assoc-in db p (handler val v))))))) +(defn undoable + "A Middleware factory which stores an undo checkpoint. + \"explanation\" can be either a string or a function. If it is a + function then it must be: (db event-vec) -> string. + \"explanation\" can be nil. in which case no + " + [explanation] + (fn middleware + [handler] + (fn undoable-handler + [db event-vec] + (let [explanation (cond + (fn? explanation) (explanation db event-vec) + (string? explanation) explanation + (nil? explanation) "" + :else (warn "re-frame: undoable given bad parameter. Got: " explanation))] + (store-now! explanation) + (handler db event-vec))))) + + (defn enrich "Middleware factory which runs a given function \"f\" in the after position. \"f\" is (db) -> db From b5ec3c6db5f277f47e9eeaccc49c70d2f8cf86f6 Mon Sep 17 00:00:00 2001 From: mike-thompson-day8 Date: Fri, 6 Mar 2015 10:48:20 +1100 Subject: [PATCH 4/7] Reimpliment undo so as to handle explanations --- src/re_frame/undo.cljs | 83 +++++++++++++++++++++++++++++++++++------- 1 file changed, 69 insertions(+), 14 deletions(-) diff --git a/src/re_frame/undo.cljs b/src/re_frame/undo.cljs index 648e4f0..5f97192 100644 --- a/src/re_frame/undo.cljs +++ b/src/re_frame/undo.cljs @@ -1,38 +1,56 @@ (ns re-frame.undo (:require-macros [reagent.ratom :refer [reaction]]) (:require + [re-frame.utils :refer [warn]] [reagent.core :as reagent] [re-frame.db :refer [app-db]] [re-frame.handlers :as handlers] - [re-frame.subs :as subs ])) + [re-frame.subs :as subs])) ;; -- History ------------------------------------------------------------------------------------- ;; ;; (def ^:private max-undos (atom 50)) ;; maximum number of undo states maintained -(defn set-max-undos +(defn set-max-undos! [n] (reset! max-undos n)) -;; + (def ^:private undo-list (reagent/atom [])) ;; a list of history states (def ^:private redo-list (reagent/atom [])) ;; a list of future states, caused by undoing +;; -- Explainations ----------------------------------------------------------- +;; +;; Each undo has an associated explanation which can be displayed to the user. +;; +;; Seems really ugly to have mirrored vectors, but ... +;; the code kinda falls out when you do. I'm feeling lazy. +(def ^:private app-explain (reagent/atom "")) ;; mirrors app-db +(def ^:private undo-explain-list (reagent/atom [])) ;; mirrors undo-liat +(def ^:private redo-explain-list (reagent/atom [])) ;; mirrors redo-list + (defn clear-history! [] (reset! undo-list []) - (reset! redo-list [])) + (reset! redo-list []) + (reset! undo-explain-list []) + (reset! redo-explain-list [])) (defn store-now! "stores the value currently in app-db, so the user can later undo" - [] + [explanation] (reset! redo-list []) ;; clear and redo state created by previous undos + (reset! redo-explain-list []) ;; clear and redo state created by previous undos (reset! undo-list (vec (take @max-undos - (conj @undo-list @app-db))))) + (conj @undo-list @app-db)))) + (reset! undo-explain-list (vec (take + @max-undos + (conj @undo-explain-list @app-explain)))) + (reset! app-explain explanation)) (defn undos? [] @@ -60,23 +78,60 @@ (reaction (redos?)))) +(subs/register + :undo-explantions + (fn handler + ; "returns a vector of string explnations + [_ _] + (reaction (deref undo-explain-list)))) + +(subs/register + :redo-explantions + (fn handler + ; "returns a vector of string explnations + [_ _] + (reaction (deref redo-explain-list)))) + ;; -- event handlers ---------------------------------------------------------------------------- + +(defn undo + [undos cur redos] + (let [u @undos + r (cons @cur @redos)] + (reset! cur (last u)) + (reset! redos r) + (reset! undos (pop u)))) + + (handlers/register-base ;; not a pure handler :undo ;; usage: (dispatch [:undo]) (fn handler [_ _] - (when (undos?) - (reset! redo-list (cons @app-db @redo-list)) - (reset! app-db (last @undo-list)) - (reset! undo-list (pop @undo-list))))) + (if-not (undos?) + (warn "re-frame: you did a (dispatch [:undo]), but there is nothing to undo.") + (do + (undo undo-list app-db redo-list) + (undo undo-explain-list app-explain redo-explain-list))))) + + +(defn- redo + [undos cur redos] + (let [u (conj @undos @cur) + r @redos] + (reset! cur (first r)) + (reset! redos (rest r)) + (reset! undos u))) + (handlers/register-base ;; not a pure handler :redo ;; usage: (dispatch [:redo]) (fn handler [_ _] - (when (redos?) - (reset! app-db (first @redo-list)) - (reset! redo-list (rest @redo-list)) - (reset! undo-list (conj @undo-list @app-db))))) + (if-not (redos?) + (warn "re-frame: you did a (dispatch [:redo]), but there is nothing to redo.") + (do + (redo undo-list app-db redo-list) + (redo undo-explain-list app-explain redo-explain-list))))) + From f0fbea41a915f36c82cb3d589346b5eabdd073c2 Mon Sep 17 00:00:00 2001 From: mike-thompson-day8 Date: Fri, 6 Mar 2015 11:40:30 +1100 Subject: [PATCH 5/7] Various --- examples/todomvc/src/todomvc/handlers.cljs | 6 +++--- src/re_frame/middleware.cljs | 4 ++-- src/re_frame/undo.cljs | 1 - 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/examples/todomvc/src/todomvc/handlers.cljs b/examples/todomvc/src/todomvc/handlers.cljs index 2feb9b9..67a4fb7 100644 --- a/examples/todomvc/src/todomvc/handlers.cljs +++ b/examples/todomvc/src/todomvc/handlers.cljs @@ -34,15 +34,15 @@ check-schema ;; middleware (fn [_ _] ;; the handler (merge default-value - (get-local-storage)))) ;; all hail the new state + #_(get-local-storage)))) ;; all hail the new state (register-handler ;; handlers changes the footer filter :set-showing ;; event-id - [write-ls check-schema debug trim-v] ;; middleware (wraps the handler) + [(path [:showing]) #_write-ls check-schema debug trim-v] ;; middleware (wraps the handler) (fn ;; handler [db [filter-kw]] - (assoc db :showing filter-kw))) + filter-kw)) (register-handler ;; given the text, create a new todo diff --git a/src/re_frame/middleware.cljs b/src/re_frame/middleware.cljs index 342dffb..cc74998 100644 --- a/src/re_frame/middleware.cljs +++ b/src/re_frame/middleware.cljs @@ -75,7 +75,7 @@ If a get-in of the path results in a nil, then \"default-fn\" will be called to supply a value. XXX very like update-in. Should the name be more indicative of that closeness? " ([p] - (path p #(nil))) + (path p nil)) ([p default-fn] (fn middleware [handler] @@ -84,7 +84,7 @@ (if-not (vector? p) (warn "re-frame: \"path\" expected a vector, got: " p)) (let [val (get-in db p) - val (if (nil? val) (default-fn) val)] + val (if (and (nil? val) (fn? default-fn)) (default-fn) val)] (assoc-in db p (handler val v))))))) diff --git a/src/re_frame/undo.cljs b/src/re_frame/undo.cljs index 5f97192..88ada0f 100644 --- a/src/re_frame/undo.cljs +++ b/src/re_frame/undo.cljs @@ -60,7 +60,6 @@ [] (pos? (count @redo-list))) - ;; -- subscriptions ----------------------------------------------------------------------------- (subs/register From a180df85c8d3182adab060cabd0ddb2a61d5b3ed Mon Sep 17 00:00:00 2001 From: mike-thompson-day8 Date: Fri, 6 Mar 2015 12:44:22 +1100 Subject: [PATCH 6/7] core.sync completely trashes the stack in an exception. Print any exception to the console before that happens. --- src/re_frame/handlers.cljs | 10 +++++++++- src/re_frame/router.cljs | 14 ++++++++------ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/re_frame/handlers.cljs b/src/re_frame/handlers.cljs index cc8d165..1077b9d 100644 --- a/src/re_frame/handlers.cljs +++ b/src/re_frame/handlers.cljs @@ -65,5 +65,13 @@ handler-fn (lookup-handler event-id)] (if (nil? handler-fn) (warn "re-frame: no event handler registered for: \"" event-id "\". Ignoring.") ;; TODO: make exception - (handler-fn app-db event-v)))) + (try + (handler-fn app-db event-v) + (catch :default e + (do + ;; use of a core.async loop seems to completely ruin exception stacks. + ;; So we're going print the exception to the console here, before it gets trashed. + (.error js/console e) + (throw e))))))) + diff --git a/src/re_frame/router.cljs b/src/re_frame/router.cljs index 190ea2b..4d38b8e 100644 --- a/src/re_frame/router.cljs +++ b/src/re_frame/router.cljs @@ -32,10 +32,7 @@ _ (if (:flush-dom (meta event-v)) ;; check the event for metadata (do (flush) ( Date: Fri, 6 Mar 2015 13:02:30 +1100 Subject: [PATCH 7/7] Improve change log --- CHANGES.md | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index cdf28c0..e55ea94 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -34,9 +34,11 @@ Changes: ### Headline + - exceptions in handler now reported more sanely via console.error. + (core.async really messes with a good stack) - example `todomvc` available in examples folder. - Wiki documentation is now more substantial. - - introduce some new handler middleware: `debug`, `enrich` and `after` + - introduce new handler middleware: `debug`, `enrich` and `after` ### Other: - exceptions in a go loop are a special type of hell. Improve the reporting of exceptions happening in event handlers. @@ -52,19 +54,18 @@ Changes: ### Details On Undo Changes -We've made changes so that the undo/redo feature is more powerful. Associated -with each undo state is an explanation which can be presented to the user to -inform them as to the actions they will be undoing or redoing. +The undo/redo feature built into re-frame is now more functional +(at the cost of a breaking change). + +There is now an explanation associated with each undo state describing +modification. This allows an app to inform the user what actions +they will be undoing or redoing. Previously `undoable` was simply middleware, but it is now a middleware factory. -Essentially that means you can't use it "plain" anymore, you must call it to -get middleware (put it in `()`) and, when you do, supply a parameter which is -an explanation for the mutation. "Set spam flag to yes", "add todo", etc. - -When the time comes to present undos to the user, you then have an explanation -associated with each one. +Essentially, that means you can't use it "plain" anymore, and instead you must +call it, like this `(undoable "Some explanation")` The `explanation` provided to undoable must be either a `string` (static -explanation) or a function `(db event) -> string` (allowing you to customize -the undo message based on details of the event. "Added todo called blah blah blah"). +explanation) or a function `(db event) -> string`, allowing you to customize +the undo message based on details of the event.