Skip to content
This repository has been archived by the owner on Apr 20, 2018. It is now read-only.

delay loading of EventSource and WebSocket #26

Merged
merged 3 commits into from
Jul 5, 2017
Merged

delay loading of EventSource and WebSocket #26

merged 3 commits into from
Jul 5, 2017

Conversation

loomis
Copy link
Contributor

@loomis loomis commented Jul 5, 2017

Delay the loading of EventSource and WebSocket until they are used. This allows kvlt to be used on browsers that don't support them as long as the features are not actually used.

@loomis
Copy link
Contributor Author

loomis commented Jul 5, 2017

Related to #20 and #25.

@moea
Copy link
Member

moea commented Jul 5, 2017

@loomis: Thanks!

Because the single arg constructor calls are the only two times those defines are used, maybe this might be cleaner than the let?

(def eventsource (delay (js/require "eventsource")))

(defn event-source [url]
  (if (exists? js/EventSource)
    (js/EventSource. url)
    (try
     (@eventsource. url)
     (catch js/Error e
       ...))))

at least with recent clojurescript builds, this works:

cljs.user> (def x (delay js/Object))
#'cljs.user/x
cljs.user> (@x.)
#js {}

but if it doesn't, generally, then confining the local binding to that function might be cleaner.

If that's a pain for you, I can merge this into a branch and take care of it.

@loomis
Copy link
Contributor Author

loomis commented Jul 5, 2017

@moea No problem. I'll refactor the PR and retest.

@moea
Copy link
Member

moea commented Jul 5, 2017

@loomis This looks good to me, I'll merge and push an updated 0.2.0-SNAPSHOT after updating the promesa dep.

@moea moea merged commit fefe1a1 into nervous-systems:master Jul 5, 2017
@loomis
Copy link
Contributor Author

loomis commented Jul 5, 2017

@moea Thanks for the feedback. I've refactored the PR. This is working for me and the standard tests pass now. You'll notice that the only change between the last two commits was switching from:

(@websocket-node. url)

which fails the unit test to

(let [ws @websocket-node]
  (ws. url)

which passes. I don't pretend to understand why the first works fine from the repl, but fails when compiled.

@moea
Copy link
Member

moea commented Jul 5, 2017

0.1.5-SNAPSHOT, rather, as there are no breaking changes in this branch. Just released to clojars.

@loomis loomis deleted the feature/optional-es-ws branch July 6, 2017 04:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants