Fork me on GitHub
#babashka
<
2020-03-06
>
borkdude09:03:22

Added clojure.core/read-string to bb:

$ bb "(ns foo (:require [clojure.string :as str])) (read-string \"::str/foo\")"
:clojure.string/foo

👀 4
babashka 4
borkdude09:03:16

$ bb '(def f (eval (read-string "(fn [] :foo)"))) (f)'
:foo

jeroenvandijk09:03:43

btw, what do you think about adding eval to sci? It would have the same limitations as the outer evalString. Makes sense?

borkdude09:03:16

yeah, eval and read-string can probably be moved to sci. I'll do that now

borkdude09:03:47

yeah that will probably work... I'm thinking about the security issues with :deny and :allow

jeroenvandijk09:03:18

how do you mean? wouldn’t they be forwarded to the eval context?

jeroenvandijk09:03:37

so you wouldn’t be able to override it in eval. Just be able to do somethign dynamic

borkdude09:03:40

I think it will work, I'll add some tests for it

jeroenvandijk09:03:55

cool, let me know if you want me to do it

borkdude09:03:43

ok, if you read through this commit, you can see what should be moved to sci: https://github.com/borkdude/babashka/commit/aa4139931e8a66e3d1977339a08eff050d7e1aea

borkdude09:03:51

I touched both eval and read-string there

borkdude09:03:05

if you want to do it, ok with me

borkdude09:03:05

in sci this would need an additional test to see if it's safe with :deny and :allow etc.

jeroenvandijk09:03:07

ok maybe i should finish the apply stuff first, but I could do it later

borkdude09:03:46

yeah, that's probably the best to finish the in progress stuff first

jeroenvandijk10:03:35

1.5 day of work 🙂 @borkdude it’s ready for another review. Commits can be squashed probably https://github.com/borkdude/sci/pull/283

borkdude10:03:33

I'll squash all PRs anyway

borkdude10:03:14

eval + read-string are now on sci master

👀 4
jeroenvandijk10:03:00

I’ll start using read-string right away 🙂

jeroenvandijk10:03:13

i was already abusing evalString for that

jeroenvandijk10:03:01

my branch fails on Linux with No implementation of method: :unbind of protocol: #'sci.impl.vars/IVar and this happened yesterday as well. Seems to be unrelated to my work. Is there some flakyness in that test that you are aware of? https://circleci.com/gh/borkdude/sci/4056?utm_campaign=vcs-integration-link&amp;utm_medium=referral&amp;utm_source=github-build-link

jeroenvandijk10:03:44

cannot find the failed build of yesterday now, but I remember seeing this before

borkdude10:03:49

yes, that's a flakiness that's been there for a long time and I can't discover what causes this. it never happens on babashka, but it does happen on sci CI

borkdude10:03:12

maybe it's some kind of concurrency bug in the clojure Compiler, I don't know

borkdude10:03:37

since I can't discover what this is caused by, just pressing run again will likely fix it

jeroenvandijk10:03:57

ok cool good to know

jeroenvandijk10:03:07

probably a redefinition of the protocol

jeroenvandijk10:03:49

the only relevant difference between sci and babashka i can see is that Babashka defines a :main in project.clj. I’ve seen that :aot and :main make some namespaces be reloaded twice, which could cause a reloading of the namespace with the relevant protocol, which could cause this error message for all the namespaces that were loaded before the second reload. Not sure if that makes sense, but I’ve seen this type of error in the repl many times

borkdude10:03:54

feel free to experiment to make it go away 🙂

jeroenvandijk10:03:14

haha yes indeed. I’ll save it for later. Now need to get something done with all those beautiful features you have added

borkdude10:03:48

I'll take a look at the PR tonight or tomorrow again, need to work on something else now

jeroenvandijk10:03:05

perfect, no rush. I have a locally compiled version

james11:03:39

Is there a way use the babashka docker image to include deps as clojure is not installed by default?

lispyclouds13:03:34

The other way of doing it would be better depending on your problem. Use the clojure image as a base and install babashka binary only. The way that I think about these things is whats my primary use-case: if its linting before a clojrue build, id use clj as base image, if I need to use babashka to do some more fancier things, I'd use that as base etc. Hope this helps! 🙂

lispyclouds13:03:55

ah sorry, was having clj-kondo in mind 😛

lispyclouds13:03:21

execute some clj scripts before a clj build

borkdude12:03:16

@James somehow I'm not able to select your name from the dropdown

borkdude12:03:07

Anyway. The babashka docker image is really thin, based on busy-box:musl. If you want to add more stuff, it's probably better to start over from an image like ubuntu or whatever and then download the binary into the image, add clojure, etc.

👍 4
borkdude12:03:25

Or start with the clojure image, maybe easier. (cc @rahul080327 )

james12:03:33

@borkdude makes sense to me. Thanks for Babashka. It is a very nice scale-down addition to the clojure world. Lots to think about.

borkdude12:03:05

Glad you find it helpful. Keep us posted what cool things you're doing with it.

jeroenvandijk15:03:03

JS challenge no. X:

(defn constructor-fn [class]
  (let [constructor (js/Function.prototype.bind.apply class)]
    (fn
      ([a] (new constructor a))
      ([a b] (new constructor a b c))
      ([a b c] (new constructor a b c d))
      ([a b c d] (new constructor a b c d e))
      ([a b c d e] (new constructor a b c d e))
      ([a b c d e f] (new constructor a b c d e f))
      ([a b c d e f g] (new constructor a b c d e f g))
      ([a b c d e f g & more]
       (throw (ex-info "Constructors with more than 7 arguments are not supported" {:class class}))))))
(apply (constructor-fn js/EventSource) ["/yeah"])
🙈

borkdude15:03:46

I have a macro like this (but more complicated) in interpreter.cljc which does the same for function invocations

borkdude15:03:53

I think it's called fn-call

jeroenvandijk15:03:06

ah cool, i’ll have a look

jeroenvandijk15:03:24

It’s has to be runtime i think

jeroenvandijk15:03:54

it cannot be a macro, but I’ll try

borkdude15:03:01

yeah, but you can generate the constructor-fn function at compile time

borkdude15:03:33

but if you're happy with this boilerplate, that's fine too 😉

jeroenvandijk15:03:46

maybe yeah, i”m affraid new is something special, but otherwise that is way better

borkdude15:03:15

first let's make it work

jeroenvandijk15:03:29

yeah fingers crossed, stupid js

jeroenvandijk15:03:40

pooh this is so confusion haha. Looking at normal sci that looks like cljs, but in the background has those magic constructors. Let’s make this work and never think about it again

jeroenvandijk16:03:53

Is there a reason why sci fn’s are clojure fn’s and not javacscript functions? For better interop it would be easier if they were js functions (in some cases at least). Maybe a macro should be added that creates a js fn

jeroenvandijk16:03:23

E.g. this doesn’t work now because the fn is not a js fn: sci.evalString("(.addEventListener (js/EventSource. \"/aaa\") \"message\" (fn [e] (.log js/console e)))", {"classes": {"allow": "all", "js": window}})

jeroenvandijk16:03:58

sorry, I understand why fn’s are not plain js functions. I’m trying to think of a solution

borkdude17:03:58

functions returned from sci can be called in JS unless they are macros (because macros contain metadata), but toJS fixes that

borkdude17:03:48

They are normal functions just like CLJS creates them.

jeroenvandijk17:03:54

The special case works now:

sci.evalString("(js/EventSource. \"/aaa\")", {"classes": {"allow": "all", "js": window}})

jeroenvandijk17:03:23

I’m trying to figure out how to add the event listener inline. Trying toJS now

borkdude17:03:49

what is the error you are getting.

jeroenvandijk17:03:17

sci.evalString("(.addEventListener (js/EventSource. \"/aaa\") \"message\" (to-js (fn [e] (.log js/console e))))", {"classes": {"allow": "all", "js": window}})

sci.min.js.map:400 Sci received opts {:bindings {to-js #object[yW]}, :namespaces {}, :allow [], :deny [], :preset nil, :realize-max nil, :classes {allow all, js #object[Window [object Window]], :allow all}, :env nil}
analyzer.cljc:175 Uncaught Nn {message: "Illegal invocation [at line 1, column 1]", data: k, Ac: TypeError: Illegal invocation
    at Zh (…, name: "Error", description: undefined, …}
vQ @ analyzer.cljc:175
sW @ js.cljs:50
(anonymous) @ js.cljs:50
Z @ js.cljs:50
uW @ js.cljs:50
bW @ js.cljs:50
AW.b @ js.cljs:50
AW @ js.cljs:50
(anonymous) @ VM1156:1

jeroenvandijk17:03:33

I tried to add the to-js via the bindings

jeroenvandijk17:03:45

it’s the same if I don’t use to-js

jeroenvandijk17:03:03

I’m trying it in steps now to see what’s wrong

borkdude17:03:16

that's always a good idea

jeroenvandijk17:03:50

This works 🙂

sse = sci.evalString("(js/EventSource. \"/sse\")", {"classes": {"allow": "all", "js": window}})

f = sci.evalString("(fn [e] (.log js/console e))", {"classes": {"allow": "all", "js": window}})
sse.addEventListener("message", f)

jeroenvandijk17:03:55

which is already quite something

borkdude17:03:33

so maybe the function isn't properly interpreted yet and you're adding just a sexpr?

jeroenvandijk17:03:18

it would be evaluated from left to right? Wouldn’t it be interpreted as a function?

jeroenvandijk17:03:48

right to left i mean

jeroenvandijk17:03:11

I’ll start debugging

jeroenvandijk17:03:55

I’m trying this now:

sci.evalString("(.addEventListener (js/EventSource. \"/sse\") \"message\" f)", {"classes": {"allow": "all", "js": window, "bindings": {f: function(e) { console.log(e) }}}} )
gives
Could not resolve symbol: f

jeroenvandijk17:03:05

not sure what that is

jeroenvandijk17:03:46

I feel like a guinea pig 🙂

jeroenvandijk17:03:38

ah wrong brackets

jeroenvandijk17:03:11

same error, so it is something at a different layer indeed. I see that the argument to the function is wrapped (message #object[f])

jeroenvandijk17:03:21

maybe I need to unwrap all args to interop calls?

borkdude17:03:06

maybe it's similar for clj interop? I have no idea without diving into this code

jeroenvandijk17:03:45

ah yeah you call object-array. Maybe that’s the same indeed

jeroenvandijk18:03:16

Following works now , except for the read-string version

(let [sse (js/EventSource. (ext dev-url))]
        (.addEventListener sse "message"
                           (fn [e]

                             (js/console.log "Data 1" e)

                             (js/console.log "Data 2" (.-data e))
                             (js/console.log "Data 3" (eval (.-data e)))
                             (js/console.log "Data 3" (pr-str (.-data e)))

                             ;; Fails
                             (js/console.log "Data 4" (read-string (.-data e)))

                             )))

jeroenvandijk18:03:52

it says ” “No protocol method HasName.getName defined for type null: ”

jeroenvandijk18:03:00

Maybe I broke something with all the interop 🙂

jeroenvandijk18:03:14

oh the eval doesn’t work either I see now. Maybe something wrong with the context. In simple cases read-string and eval work

jeroenvandijk18:03:46

oh wait eval expects a form. Never mind

jeroenvandijk18:03:42

code is here btw https://github.com/borkdude/sci/pull/286 I’ll revisit it later. Calling it a day now. Have a good weekend!

borkdude21:03:43

Does this mean I can ignore the previous PR then? I'll wait reviewing until you are done experimenting, since it seems you are still making progress. When you're done you can explain it all to me 😉

jeroenvandijk09:03:59

Yeah probably better. Basic interop has improved. If someone needs it, they can ask or checkout that branch. For the callback interop I'm still hitting walls. Also I was thinking that I can probably create better unit tests along the way as I start to get a better idea of how all this .apply stuff in js works. Maybe the browser tests could be postponed as well

borkdude10:03:29

Yeah, good idea. Also splitting up your improvements in smaller units (PRs with code and tests) is maybe an idea, so it's easier to understand and review?

jeroenvandijk18:03:06

If I pass read-string via bindings

"read-string": function(data){ return sci.evalString(data, {}) }
it does work

jeroenvandijk18:03:14

Ok really going now 🙂

borkdude08:03:24

I've tested eval + read-string on node, it seems to work there:

evalString('(eval (read-string "(+ 1 2 3)"))');
6

jeroenvandijk09:03:05

Yeah same here. For some reason it doesn't work anymore in my js callback. I'm guessing it has to with all the js interop. I'm taking a small break on debugging. We'll look at it again next week