Skip to content

Commit

Permalink
Fix async let binding expansion
Browse files Browse the repository at this point in the history 
Previously, the `async` macro would attempt to treat a non-symbol form as a function call
and so the following would result:

```
(h/async
 (let [result (await (js/Promise.resolve "value"))]
  ...))

;; Compiles into...
var xyz = Promise.resolve("value").call()
```

This commit fixes this so that let bindings are properly created.

- Remove macroexpansion from tests

With the correct let binding handling, macroexpansion doesn't work because the resulting
binding names are random (something like v_51003, p_40892), which can't really be equated.
Since we can test the `async` macro directly we can just remove the macroexpansion tests
completely.

Fixes #30, #31.
  • Loading branch information
@jo-sm
jo-sm committed Jun 1, 2023
1 parent ffbb628 commit 49238cf
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 99 deletions.
3 changes: 1 addition & 2 deletions cljest/deps.edn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,4 @@
:publish {:extra-deps {appliedscience/deps-library {:mvn/version "0.3.4"}}}
:test {:extra-paths ["example_tests"]
:extra-deps {com.pitch/uix.core {:mvn/version "0.8.1"}
com.pitch/uix.dom {:mvn/version "0.8.1"}
net.clojars.cyrik/cljs-macroexpand {:mvn/version "0.1.1"}}}}}
com.pitch/uix.dom {:mvn/version "0.8.1"}}}}}
5 changes: 4 additions & 1 deletion cljest/src/cljest/format.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@
`(fn []
(str "Expected " ~(value->str a) " to " ~(when negated? "not ") "equal " ~(value->str b) "."
(when-not ~negated?
(str "\n\n" (cljest.auxiliary/generate-diff ~a ~b)))))))
(str "\n"
"Expected:\n" ~a "\n\n"
"Actual:\n" ~b "\n\n"
(cljest.auxiliary/generate-diff ~a ~b)))))))

(defmethod formatter 'cljs.core/not=
[_ form negated?]
Expand Down
4 changes: 2 additions & 2 deletions cljest/src/cljest/helpers/core.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@
(take-nth 2 (drop 1 bindings)))]
{:current (group)
:rest (conj rest
(group (conj forms (list 'js/Promise.all binding-vals)))
(group [(concat ['cljest.helpers.core/async] let-exprs)] binding-names))})
(group (conj forms (list 'js/Promise.all `[~@binding-vals])))
(group [(concat ['cljest.helpers.core/async] let-exprs)] [(apply vector binding-names)]))})

;; If we have `let` but there aren't any `await` calls in the binding values, just create a new
;; `async` wrapped group.
Expand Down
128 changes: 34 additions & 94 deletions cljest/src/cljest/helpers/core_test.cljs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
(ns cljest.helpers.core-test
(:require [cljest.core :refer [describe is it]]
(:require [cljest.core :refer [describe is it spy]]
[cljest.helpers.core :as h]
[cyrik.cljs-macroexpand :refer [cljs-macroexpand-all] :rename {cljs-macroexpand-all macroexpand-all}]))
[cljest.matchers :as m]))

(defn ^:private next-macrotask+
[]
(js/Promise. (fn [res _] (js/setTimeout res))))

(describe "with-mocks"
(defn ^:private cool-fn
Expand Down Expand Up @@ -49,96 +53,32 @@
(is (= -2 (something-else-stateful)))))

(describe "async"
(it "should macroexpand into a resolves promise when called with nothing"
(is (= (macroexpand-all '(js/Promise.resolve))
(macroexpand-all '(h/async)))))

(it "should add the provided form to the body of the `then` function"
(is (= (macroexpand-all '(-> (js/Promise.resolve)
(.then (fn []
(fn-1)
(fn-2 with-an-arg)))))
(macroexpand-all '(h/async
(fn-1)
(fn-2 with-an-arg))))))

(it "should handle the `await` keyword by separating the bodies with then"
(is (= (macroexpand-all '(-> (js/Promise.resolve)
(.then (fn []
(fn-1)
(async-fn-2)))
(.then (fn []
(fn-3 with-an-arg)))))

(macroexpand-all '(h/async
(fn-1)
(await (async-fn-2))
(fn-3 with-an-arg))))))

(it "should handle `await` inside of `let` and turn the let body into another `async` call"
(is (= (macroexpand-all '(-> (js/Promise.resolve)
(.then (fn []
(async-fn-1)))
(.then (fn []
(let [name-1 :kw]
(-> (js/Promise.resolve)
(.then (fn []
(fn-2)
(async-fn-3 name-1)))))))
(.then (fn []
(fn-4)))))

(macroexpand-all '(h/async (await (async-fn-1))
(let [name-1 :kw]
(fn-2)
(await (async-fn-3 name-1)))
(fn-4))))))

(it "should turn await inside of a let binding value into Promise.all"
(is (= (macroexpand-all '(-> (js/Promise.resolve)
(.then (fn []
(fn-1)
(js/Promise.all [promise-1 non-promise promise-2])))
(.then (fn [name-1 name-2 name-3]
(-> (js/Promise.resolve)
(.then (fn []
(fn-2)
(async-fn-3 name-3))))))
(.then (fn []
(fn-4)))))

(macroexpand-all '(h/async (fn-1)
(let [name-1 (await promise-1)
name-2 non-promise
name-3 (await promise-2)]
(fn-2)
(await (async-fn-3 name-3)))
(fn-4))))))
(it "should handle nested lets"
(is (= (macroexpand-all '(-> (js/Promise.resolve)
(.then (fn []
(async-fn-1)))
(.then (fn []
(fn-2)
(let [name-1 :kw]
(-> (js/Promise.resolve)
(.then (fn []
(fn-3 name-1)
(js/Promise.all [:kw-1 promise-1])))
(.then (fn [name-1-1 name-2-2]
(-> (js/Promise.resolve)
(.then (fn []
(fn-4 name-2-2))))))))))

(.then (fn []
(fn-5)))))

(macroexpand-all '(h/async (await (async-fn-1))
(fn-2)