scittle 2022-10-18 | Slack Archive

craftybones02:10:32

@borkdude - on scittle.nrepl there's some code that checks to see if SCITTLE_NREPL_WEBSOCKET_PORT is set and if so it sets the ws_nrepl variable. https://github.com/babashka/scittle/blob/12e5a33964e0dd8feba8537c62867abb3aa5e81a/src/scittle/nrepl.cljs#L34

craftybones02:10:46

shouldn't

(new js/WebSocket "")

(new js/WebSocket (str ":" (.-SCITTLE_NREPL_WEBSOCKET_PORT js/window) "/_nrepl")

craftybones02:10:26

That port var isn't getting used anywhere else

craftybones02:10:21

I'm not running it, just browsing through and it seemed wrong

borkdude09:10:37

maybe yes

borkdude09:10:47

please try it out and submit a PR if it turns out to be true

craftybones13:10:30

I've confirmed that it is broken and I've fixed it. However, I have a question. For the example to be fixed, a dist needs to be pushed with potentially a version bump. So this is going to have to happen in two stages. First a PR to merge the changes and generate a dist. Then once that dist is pushed to CDN, change the example to reflect the correct version. Is that fine?

borkdude13:10:20

sure

craftybones14:10:37

I've raised a PR. https://github.com/babashka/scittle/pull/45

craftybones14:10:42

Kindly approve.

borkdude14:10:19

Two comments

craftybones14:10:32

I've made the relevant changes. Used a when-let and tested it in prod and dev mode.

craftybones14:10:42

I've removed the deref as well.