Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix async let binding expansion #35

Merged
merged 4 commits into from
Jun 9, 2023
Merged

Fix async let binding expansion #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d2fc1eb
Fix async let binding expansion
jo-sm Jun 1, 2023
b457de2
Properly handle `let` bindings.
jo-sm Jun 5, 2023
49e9b75
Update changelog
jo-sm Jun 5, 2023
230eb22
Remove debug info from format
jo-sm Jun 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
- [Always reinstantiate mocks for each test case when using `setup-mocks`.](https://github.com/pitch-io/cljest/pull/28)
- [Fix matcher negation.](https://github.com/pitch-io/cljest/pull/27)
- [Correctly determine the files to run based off of the namespace regex.](https://github.com/pitch-io/cljest/pull/33)

- [Properly handle an `await`-ed `let` binding value](https://github.com/pitch-io/cljest/pull/35). Fixes [#31](https://github.com/pitch-io/cljest/issues/31)
- [Correctly handle multiple binding pairs inside of `let`](https://github.com/pitch-io/cljest/pull/35). Fixes [#30](https://github.com/pitch-io/cljest/issues/30).

# 1.0.0

Expand Down
140 changes: 87 additions & 53 deletions cljest/src/cljest/helpers/core.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,85 @@
(.finally finish#))))

(defn ^:private group
"Creates a new \"group\" map that has `forms` and `bindings` keys."
"Creates a new 'group' map that has `forms` and `bindings` keys."
([] (group [] []))
([forms] (group forms []))
([forms bindings] {:forms forms :bindings bindings}))

(defn ^:private await-seq?
"If the given `form`, as a list of quoted symbols, is eqv to `(list 'await ...)`"
(defn ^:private await-form?
"Returns true if the given `form` is a sequence and the first element is `'await`."
[form]
(and (seq? form) (= 'await (first form))))

(defn ^:private let-form?
"Returns true if the given `form` is a sequence and the first element is `'let`."
[form]
(and (seq? form) (= 'let (first form))))

(defn ^:private unwrap-await
"If `form` is an `await`-wrapped form, return what it is wrapping."
[form]
(if (await-form? form)
(second form)
form))

(defn ^:private forms->groups
"Takes all `forms` inside of an `async` block and turns them into `groups`, which later get turned into `.then`
calls."
[forms]
(let [{:keys [current prev]}
(reduce
(fn [{prev :prev {:keys [forms bindings]} :current} form]
(cond
;; If the form itself is `(await ...)`, take the inner part and add it to the current group,
;; then add the current group to the `prev` sequence.
(await-form? form)
{:current (group)
:prev (conj prev
(group (conj forms (second form)) bindings))}

;; If the form is `let` and any of the binding values has `(await ...)`, take the first binding pair
;; and use its value as the return value of the current group. Add a new group with the symbol as the
;; first binding.
(and (let-form? form) (some await-form? (second form)))
(let [all-bindings (second form)
first-binding-sym (first all-bindings)
first-binding-val (unwrap-await (second all-bindings))
rest-bindings (nthrest all-bindings 2)
let-forms (nthrest form 2)

;; If there aren't any more bindings (which would generate `(let [] forms)`),
;; use `forms` instead of creating another `let`.
next-async-expr (if (empty? rest-bindings)
(concat ['cljest.helpers.core/async] let-forms)
(list 'cljest.helpers.core/async (concat (list 'let rest-bindings) let-forms)))]
{:current (group)
:prev (conj prev
(group (conj forms first-binding-val))
(group [next-async-expr] [first-binding-sym]))})

;; If the form is `let` but there aren't any `await` calls in the binding values, just create a new
;; `async` wrapped group.
(let-form? form)
{:current (group)
:prev (conj prev
(group
(conj forms (list 'let (second form) (concat ['cljest.helpers.core/async] (nthrest form 2))))
bindings))}

;; Otherwise, add the current form to the current group's forms.
:else
{:current (group (conj forms form) bindings)
:prev prev}))
{:current (group)
:prev []}
forms)]

;; Prevent unnecessary functions from being added to the result
(if (empty? (:forms current))
prev
(conj prev current))))

(defmacro async
"Similar to JS's async/await. Wraps the body of `async` in a promise and allows for the use
of `await`, which when called will wait for the promise to finish before continuing execution
Expand Down Expand Up @@ -94,55 +163,20 @@
(some-fn)))
```
"
[& body]
(let [then-groups (->> body
(reduce
(fn [{rest :rest {:keys [forms bindings]} :current} form]
(cond
;; If the form itself is `(await ...)`, take the `...`, add it to the current group,
;; and add the current group to the `rest`.
(await-seq? form)
{:current (group)
:rest (conj rest (group (conj forms (second form)) bindings))}

;; If the form is `let` and any of the binding values has `(await ...)`, add a new
;; `js/Promise.all` to the rest and add the body of the `let` as a second new group,
;; wrapped in `async`, with the binding names from the `let` as the arguments of the
;; `.then` function.
(and (= 'let (first form)) (some await-seq? (second form)))
(let [bindings (second form)
let-exprs (nthrest form 2)
binding-names (take-nth 2 bindings)
binding-vals (map
#(if (await-seq? %) (second %) %)
(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))})

;; If we have `let` but there aren't any `await` calls in the binding values, just create a new
;; `async` wrapped group.
(= 'let (first form))
{:current (group)
:rest (conj rest
(group
(conj forms (list 'let (second form) (concat ['cljest.helpers.core/async] (nthrest form 2))))
bindings))}

;; Otherwise, add the current form to the current group's forms.
:else
{:current (group (conj forms form) bindings)
:rest rest}))
{:current (group)
:rest []})

;; Prevent unnecessary functions from being added to the result
((fn [{:keys [current rest]}]
(if (empty? (:forms current))
rest
(conj rest current))))
[& forms]
(let [groups (forms->groups forms)
;; We can assume there are no bindings for the first group, since bindings necessarily must come
;; from a previous group.
first-forms (:forms (first groups))
then-groups (->> groups
rest
(map (fn [{:keys [forms bindings]}]
(list '.then (concat (list 'fn (apply vector bindings)) forms)))))]
`(-> (js/Promise.resolve)
~@then-groups)))

`(do ~@(butlast first-forms)
(let [beginning# ~(last first-forms)]
;; This avoids creating a new Promise instance if `beginning#` is thennable
(-> (if (and beginning# (.-then beginning#))
beginning#
(js/Promise.resolve beginning#))
~@then-groups)))))
133 changes: 39 additions & 94 deletions cljest/src/cljest/helpers/core_test.cljs
View file
Open in desktop
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,37 @@
(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)
(let [name-1 :kw]
(fn-3 name-1)
(let [name-1-1 :kw-1
name-2-2 (await promise-1)]
(fn-4 name-2-2)))
(fn-5)))))))
(it "should support basic `await` usage"
(let [cb (spy)
timer (js/setInterval cb)]
(h/async
(is (m/called-times? cb 0))

(await (next-macrotask+))

(is (m/called-times? cb 1))

(await (next-macrotask+))

(is (m/called-times? cb 2))

(js/clearInterval timer))))

(it "should return a promise even with a non-promise value"
(js/expect.assertions 1)

(.. (h/async 7)
(then (fn [resolved]
(is (= 7 resolved))))))

(it "should support arbitrary `await`-ed bindings inside of `let`"
(h/async
(let [value-1 {:a-key {:b-key "yeah dude"}}
value-2 (await (js/Promise.resolve value-1))
{value-3 :a-key} value-2
{value-4 :b-key} (await (js/Promise.resolve value-3))]

(is (= {:a-key {:b-key "yeah dude"}} value-1))
(is (= {:a-key {:b-key "yeah dude"}} value-2))
(is (= {:b-key "yeah dude"} value-3))
(is (= "yeah dude" value-4))))))