-
Notifications
You must be signed in to change notification settings - Fork 35
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.
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
Fixing Uncaught TypeError: Cannot read property 'autoReconnect' of null (and add deps.edn) #75
base: master
Are you sure you want to change the base?
Conversation
@@ -20,7 +20,7 @@ | |||
([auto-reconnect?] | |||
(websocket-connection auto-reconnect? nil)) | |||
([auto-reconnect? next-reconnect-fn] | |||
(goog.net.WebSocket. auto-reconnect? next-reconnect-fn))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great and obviously would work but have you also considered the following?
(goog.net.WebSocket. (js-obj "autoReconnect" auto-reconnect?) next-reconnect-fn)
I ran into the same error after upgrading some project dependencies for JDK 11 support, and the fix to WebSocket instantiation resolved the error for me. Looks like the other proposed fix should work as well, happy to test and roll another pull request for that if preferred. |
I'm not familiar with the codebase, so I'm down with whatever people feel is best. |
@@ -20,7 +20,7 @@ | |||
([auto-reconnect?] | |||
(websocket-connection auto-reconnect? nil)) | |||
([auto-reconnect? next-reconnect-fn] | |||
(goog.net.WebSocket. auto-reconnect? next-reconnect-fn))) | |||
(goog.net.WebSocket. (or auto-reconnect? false) (or next-reconnect-fn false)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to me those can be some let-bindings instead, as the code looks a bit weird as proposed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, and false
doesn't seem to be valid according to https://google.github.io/closure-library/api/goog.net.WebSocket.Options.html
maybe
(goog.net.WebSocket. (clj->js
(cond-> {}
auto-reconnect? (assoc "autoReconnect" auto-reconnect?)
next-reconnect-fn (assoc "getNextReconnect" next-reconnect-fn))))
or, to make things clearer, using something like assoc-some
from https://github.com/weavejester/medley/blob/1.3.0/src/medley/core.cljc#L38
When we are trying to instantiate and goog.net.WebSocket object, it breaks with nil/null option.
So I pass
false
when params are nil.I also added
deps.edn
to make it simpler to use:git
with a specific sha in deps.edn