From 23bec7de24ac669911c40fb77662525daef08834 Mon Sep 17 00:00:00 2001 From: Joshua Smock Date: Thu, 1 Jun 2023 18:47:35 +0200 Subject: [PATCH] Fix async let binding expansion Previously, the `async` macro would attempt to treat a non-symbol value for a binding in a `let` as a function call and so the following would happen: ``` (h/async (let [result (await (js/Promise.resolve "value"))] ...)) ;; Compiles into... var xyz = Promise.resolve("value").call() ``` This commit fixes this so that `await`-ed `let` bindings are properly handled. - 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, fixes #31. --- cljest/deps.edn | 3 +- cljest/src/cljest/format.clj | 5 +- cljest/src/cljest/helpers/core.clj | 4 +- cljest/src/cljest/helpers/core_test.cljs | 128 ++++++----------------- 4 files changed, 41 insertions(+), 99 deletions(-) diff --git a/cljest/deps.edn b/cljest/deps.edn index 7c88a4b..e1b3da2 100644 --- a/cljest/deps.edn +++ b/cljest/deps.edn @@ -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"}}}}} diff --git a/cljest/src/cljest/format.clj b/cljest/src/cljest/format.clj index bd0c9bb..14dbfda 100644 --- a/cljest/src/cljest/format.clj +++ b/cljest/src/cljest/format.clj @@ -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?] diff --git a/cljest/src/cljest/helpers/core.clj b/cljest/src/cljest/helpers/core.clj index 19bdda7..59613d5 100644 --- a/cljest/src/cljest/helpers/core.clj +++ b/cljest/src/cljest/helpers/core.clj @@ -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. diff --git a/cljest/src/cljest/helpers/core_test.cljs b/cljest/src/cljest/helpers/core_test.cljs index 1f7d97f..3985772 100644 --- a/cljest/src/cljest/helpers/core_test.cljs +++ b/cljest/src/cljest/helpers/core_test.cljs @@ -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 @@ -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) - (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 support resolved bindings" + (h/async + (let [result (await (js/Promise.resolve "cool stuff!"))] + (is (= result "cool stuff!"))))) + (it "should support multiple resolved bindings" + (h/async + (let [value-1 (await (js/Promise.resolve "cool stuff!")) + value-2 "some-non-promise" + value-3 (await (js/Promise.resolve "more cool stuff!"))] + (is (= value-1 "cool stuff!")) + (is (= value-2 "some-non-promise")) + (is (= value-3 "more cool stuff!"))))))