Fork me on GitHub
#babashka
<
2020-03-05
>
jeroenvandijk09:03:16

Btw I found something about a Truffle interpreter in Clojure http://epub.jku.at/obvulihs/download/pdf/501665 I think I’ll ask him for an opinion on Sci

borkdude09:03:03

One of the questions about Truffle things is if it would allow evaluating code at runtime in a natively compiled binary. Truffle is nice for interop in a shared polyglot runtime, but this is a different thing from evaluating programs from strings at runtime in a binary afaik.

jeroenvandijk09:03:37

Good point indeed

jeroenvandijk10:03:50

@sogaiu I randomly googled “truffle clojure” and I found that thesis and also this twitter thread https://twitter.com/alandipert/status/650843148986527744

sogaiu10:03:08

great 🙂

sogaiu10:03:11

ah, it's from 2015

jeroenvandijk10:03:11

I read a post somewhere by the GraalVM people that it was based on an older version of Truffle

sogaiu10:03:18

totally didn't know

sogaiu10:03:36

i failed to notice any dates on the other pdf

sogaiu10:03:52

but the one you gave a link to does have a date as well as the twitter page

jeroenvandijk10:03:09

I think that’s the proposal for the thesis that was written in 2015, so must be close to that

sogaiu10:03:19

makes sense

borkdude10:03:56

PR to clj-graal-docs welcome

borkdude10:03:05

although that site is mainly about native-image

jeroenvandijk12:03:29

ok, I think I have found a solution for one of the two issues in Sci js

bananadance 4
jeroenvandijk14:03:38

both issues fixed (tests pass at least), plus another one I found: https://github.com/borkdude/sci/pull/282

borkdude14:03:54

thanks, I'll take a look later tonight or tomorrow

jeroenvandijk15:03:12

Does anyone see what is wrong with this? (trying from a browser console):

sci.evalString('(js/alert "foo")', {"classes": {"js": window, "allow": "all"}, "allow": "all" })
Gives
message: "js/alert is not allowed! [at line 1, column 2]"
(i added allow at multiple places to prevent this error)

jeroenvandijk15:03:06

Maybe {"allow": "all"} is not the same as the internal {:allow :all}

borkdude15:03:42

maybe :allow all isn't parted by the JS interface

borkdude15:03:04

I think the "all" keyword might not be converted

jeroenvandijk15:03:06

Yeah could be. I’ll try to make a debug version of Sci to confirm this

borkdude15:03:56

it's an undocumented workaround anyway, might change it, since there might be a class called "allow" in JS which would then clash

borkdude15:03:33

maybe it has to be a namespaced key that has a truthy value, so anything except false or null will work

borkdude15:03:56

"sci/unchecked-interop" or something, don't know

borkdude15:03:57

the checking is done for two reasons: 1) you might want to limit access to a class. 2) when there is no check in GraalVM you just get some weird exception because the reflection information is missing

jeroenvandijk15:03:54

yeah the check makes sense, in js probably as well. I’ll try to debug it

jeroenvandijk17:03:39

made another pull request with this addition

jeroenvandijk17:03:07

I guess browser tests are needed 😬

borkdude17:03:54

we have node tests, why is that not sufficient? I'm not sure if I want to take on a dependency. can you just copy/inline that call function from the lib if needed?

jeroenvandijk17:03:27

yeah thought about that, but it is not a oneliner so I decided to add the dependency

jeroenvandijk17:03:46

It has several layers

jeroenvandijk17:03:00

Either way it’s a proof that it can work

jeroenvandijk17:03:10

for me at least 🙂

jeroenvandijk17:03:32

I can also fork Sci and make a browser version

borkdude17:03:46

ok, if they handle a lot of gnarly stuff with js interop, maybe it's worth adding

borkdude17:03:25

browser tests are fine, but also brittle. I have browser tests for https://github.com/borkdude/re-find.web. maybe you can take a look at those

jeroenvandijk17:03:28

Maybe the actual thing that is needed is actually smaller, but I am happy that I finally made it work

borkdude17:03:36

that get macro can probably just be replaced by goog.object/get

borkdude17:03:06

could you try that?

borkdude17:03:48

(.apply (goog.object/get obj method-name) obj (to-array args))

borkdude17:03:14

we're not dealing with keywords there anyway

borkdude17:03:17

only strings

jeroenvandijk17:03:29

yep, trying now

jeroenvandijk17:03:46

ok seems to work as well 🙂

jeroenvandijk17:03:52

I removed the js-interop dep again

borkdude17:03:12

so there is a special case for js/window?

borkdude17:03:20

(gobj/get class method-name) that expression is already there, it's called method in the if-let

borkdude17:03:07

I think this is what I had before. So some methods need also the class, but some don't. I wonder how one can determine that.

borkdude17:03:01

anyway, great that you made some progress

jeroenvandijk17:03:53

yeah I guess it can be cleaned up. Ok i have to go. Will look into it later

borkdude09:03:20

OK, let me know when you think it's ready for merge.

👍 4
jeroenvandijk09:03:26

btw, probably why apply doesn’t always work is described here https://medium.com/@jhawleypeters/javascript-call-vs-apply-vs-bind-61447bc5e989 AKA the horror of javascript

borkdude09:03:33

yeah... I think I hit that before without knowing it. maybe it's better to avoid this crap altogether

jeroenvandijk09:03:03

Hopefully we can fix it once and never have to deal with it again 🙌

borkdude09:03:00

We need theller or dnolen 😉

borkdude09:03:31

maybe the CLJS compiler offers some insights into this interop?

jeroenvandijk09:03:53

pooh yeah probably. I cannot go into this rabbithole 🙈 I just want some clojure in my browser haha. But good news, the apply thing seems to work for now. Browser tests will definitely help to iterate faster I’m copy pasting the minified thing to my project now and do manual tests

borkdude10:03:18

maybe that's a nice next issue to work on, browser tests