Don't evaluate default-batched-updates until necessary

bfontaine commented 5 years ago

The current code uses a def, meaning that the map will always be evaluated at run time even if you provide your own functions.

js/requestAnimationFrame and js/cancelAnimationFrame only exist in the browser, so this fails if you run the code with Node:

/home/baptiste/.boot/cache/tmp/home/baptiste/myorg/myapp/8mr/-ueasqk/cljs_test/generated_test_suite.out/citrus/core.js:6
citrus.core._default_batched_updates = new cljs.core.PersistentArrayMap(null, 2, [new cljs.core.Keyword(null,"schedule-fn","schedule-fn",1204822075),requestAnimationFrame,new cljs.core.Keyword(null,"release-fn","release-fn",-1727471084),cancelAnimationFrame], null);
                                                                                                                                                     ^

ReferenceError: requestAnimationFrame is not defined

I used an or in order to only evaluate (-get-default-batched-updates) if batched-updates is not defined. This allows one to provide custom batched-updates to run code on Node.

Note let’s :or doesn’t work because it always evaluates its default argument:

(let [{:keys [a]
       :or {a (println "hey")}} {:a 42}]
  a)
; "hey"
; => 42

roman01la commented 5 years ago

LGTM. Thanks!

bfontaine commented 5 years ago

When do you plan to make the next release?

DjebbZ commented 5 years ago

If I may, I'd like a release for this too. It's preventing us from running the tests on Node.js.

roman01la commented 5 years ago

Bumped to 3.2.3

@slipset Could you please make a release to Clojars?

slipset commented 5 years ago

Done!

DjebbZ commented 5 years ago

Thank you :+1:

On Sun, Apr 14, 2019, 10:41 AM Erik Assum notifications@github.com wrote:

Done!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/clj-commons/citrus/pull/44#issuecomment-482933393, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEt3nnRgxEiGML_0cc9RUpeLS8h2hCUks5vgunRgaJpZM4ccc8- .

DjebbZ commented 5 years ago

By the way, the next step would be to add a CI with Travis that runs the tests. The problem would have have been caught earlier. I'll see if I can do something about it.

slipset commented 5 years ago

Most of the other projects are on CircleCi, so unless you hav a good reason for using Travis, please consider using Circle.

Erik. -- i farta

apr. 2019 kl. 14:38 skrev Khalid Jebbari notifications@github.com:

By the way, the next step would be to add a CI with Travis that runs the tests. The problem would have have been caught earlier. I'll see if I can do something about it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

DjebbZ commented 5 years ago

Sure, I just meant some CI. Thanks for the pointer

On Sun, Apr 14, 2019, 2:39 PM Erik Assum notifications@github.com wrote:

Most of the other projects are on CircleCi, so unless you hav a good reason for using Travis, please consider using Circle.

Erik.

i farta

apr. 2019 kl. 14:38 skrev Khalid Jebbari notifications@github.com:

By the way, the next step would be to add a CI with Travis that runs the tests. The problem would have have been caught earlier. I'll see if I can do something about it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/clj-commons/citrus/pull/44#issuecomment-482966514, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEt3sl6D3TW3fs1Im58dSUxBjvKukhjks5vgyGXgaJpZM4ccc8- .

clj-commons / citrus

Don't evaluate default-batched-updates until necessary #44

Erik.