ok, my PR is ready for review

also: it may be worth adding a prettierjs config to the project and to use that for formatting code.

provocative question: should we be defining conj/assoc/get/etc. as protocols instead?

I mean it would sure simplify some things

I didn't think that was an option here

I pushed up some extras on that PR

protocols don't work yet

and it generates a LOT of code

it would be so nice if we could extend core types

yeah I'm thinking that we could keep the existing impls and add a protocol that it dispatches for custom types

the "don't extend built in types" JS thing makes all this business a real PITA

it really does

we could do like some libraries and have a wrap/unwrap thing

where we wrap values with our own objects for the duration of the function in order to simplify the impl

it doesn't solve the whole custom type dispatch issue

like I have this:

if (m instanceof Map) {
    m.set(k, v);

    for (let i = 0; i < kvs.length; i += 2) {
      m.set(kvs[i], kvs[i + 1]);
    }
  } else if (m instanceof Object) {
    m[k] = v;

    for (let i = 0; i < kvs.length; i += 2) {
      m[kvs[i]] = kvs[i + 1];
    }

sure would be nice to not have to do that

if it weren't for the fact we can't extend built ins, then a protocol could just be:

(defprotocol IConj
  (conj [o x]))

let IConj_conj = Symbol.for("clavascript.core.IConj/conj");

function conj(o, x) {
  let m = o[IConj_conj];
  if (!m) throw new Error("Object does not implement conj");
  return m(o, x);
}

https://gist.github.com/modernserf/13846736109de95797d1#interfaces-and-protocols-part-ii

it still extends core types but it does it in a way that cannot conflict with other things

which you have there

yeah that's p similar

I think the argument against this is that it's possible that a JS environment could freeze the built in types

to prevent prototype pollution attacks

I have never seen this in practice

I really wish TC39 would address this use case officially

> Map[Symbol.hasInstance](new Map())
true
> Map[Symbol.hasInstance]([])
false
> Map[Symbol.hasInstance]({})
false

turns out this is a built-in example of this working

but you're right that they could freeze things

ah, Symbol.species is another

I always wondered what that was about on MDN

https://gist.github.com/corasaurus-hex/6d3dfec911d5a4adeaa3f59d77e518a5

it's function by function so not quite what we need but

About the above: I don't think we have to cover CLJS behavior 100%. E.g. (conj nil ...) could just not be supported: our contract is that conj works on arrays, objects and Maps, nothing else for example. I prefer good and fast support for built-in types rather than making things more complicated and slower to support conj on custom types.

Also, I think I'd prefer writing some gnarly boilerplate if that means we can have pretty CLJS code which compiles to fast and compact JS.

That said, protocols do already work:

(ns foo (:require ["clavascript/core.js" :refer [nil_QMARK_ not PROTOCOL_SENTINEL]]))

(defprotocol IFoo (foo [_]))
(deftype Foo [x] IFoo (foo [_] :foo))
(js/alert (foo (->Foo 1)))

There's just an issue with the automatic imports not properly working for some reason which is why you need the clavascript/core.js import now

About prettier: why is let res = f(ctr,x); better than let res = f(ctr, x);?

I prefer to work in small PRs that don't add a ton of stuff in one go, so it's easier to review

I'll add a PR template for this

E.g. about assoc-in we have a separate issue and I think we should first discuss if assoc-in should make clones of each object, or update them in place. This is an important decision that needs some thought instead of slipping it through in a PR I think

What I wonder about the above changes:

would it be faster if you check .length before you go into a for (const x = ...) loop? This is what was done before in assoc Also I wonder if writing for (x = 0; x < ..; x++) ... is faster than for (const x = ...)

I think we can safely say that using the imperative counter is way faster: https://jsbench.me/cbl6ujmp3d/1

@corasaurus-hex @lilactown I appreciate your involvement in this project a lot

https://twitter.com/borkdude/status/1559165450054950914

How do you call the clavascript community? The enclava :drum_with_drumsticks:

What's the clava compiler invoked from bb called? baclava :drum_with_drumsticks:

So this defprotocol example works in the https://clavascript.github.io/clavascript/:

(defprotocol IFoo (foo [_]))
(deftype Foo [x] IFoo (foo [_] :foo))
(js/alert (foo (->Foo 1)))

Also this works:

(extend-type string IFoo (foo [_] :dude))
(js/alert (foo "dude"))

drat, enclava is already taken as an org name on github ;)

womp womp 😂

so do you think we should be building core functions in terms of protocols?

well, I think we should focus on performance first and start supporting the basic data structures JS has: object, array, Map and Set

and then it if turns out to be very useful, we could introduce protocol for assoc, etc

which then dispatches on the user implementation for the rest of the cases

unless you can convince me that a protocol dispatch on object and array is very cheap with a benchmark :)

there's just no way protocols could be cheaper than going with direct and imperative code

so I'll stay the course :)

indeed. so I think we should have direct imperative code for the most common cases and then have a fallback on a theoretical future protocol

balaclava

@corasaurus-hex I merged the assoc-in pr, but I think this is unexpected:

(js/alert (pr-str (assoc-in {} [:foo :bar] 2)))

This also raises the question about what should happen for assoc-in! for unpopulated nestings, probably create a new map

Probably we should then also support nil to assoc (but what about assoc! ?)

a new map or a new object?

I'm working on a fix now. What I'm doing is it will depend on the top level object

That will lead to this behavior:

(is (eq #js {"0" #js {"1" 2}, "1" "foo"} (jsv! '(assoc-in {"1" "foo"} [0 1] 2))))

Perhaps if the key is a number, we should default to array

it's all so weird 😅

generating an empty array is weird since you can't assoc-in there with numbers ;)

https://github.com/clavascript/clavascript/commit/a811fb92578501220134a677796e4d9d61df1539

> !0
true
> !1
false
> !2
false

the if (!baseType) is going to do something you don't intend because 0 is falsey in js

@corasaurus-hex good catch, I'll start from 1 then ;)

🙂

I also refactored assoc-in and assoc-in! to use the same code almost: https://github.com/clavascript/clavascript/commit/9e1fbbca8ff4443925b40f3c292f820e5b389d21 Hopefully I didn't miss anything

sorry I didn't hop onto this. I woke up really sick today

oh, no problem at all!

we're all just having fun I think Cora. I hope you feel better

thanks 💜

https://github.com/clavascript/clavascript/blob/9e1fbbca8ff4443925b40f3c292f820e5b389d21/core.js#L75

I think the error message needs to be adjusted based on which function is calling it

true, I'll fix that

fixed

also you mentioned maybe not handling nil or underfined as the first argument to conj https://github.com/clavascript/clavascript/blob/9e1fbbca8ff4443925b40f3c292f820e5b389d21/core.js#L143-L145

not sure if you still feel that way or not

with these type number we can now write:

switch (typeConst(x)) {
  case MAP_TYPE: ...
  case ARRAY_TYPE ...
}

it would be nice to bench using a map of functions vs a switch/case

a real map, not an object, so you can use numbers as keys

or heck, an array

I'm afraid a map of functions will not make treeshaking work very well

ahhh ok

the tension of bundle size vs performance. js is fun

OK, try (js/alert (pr-str (assoc-in {} [:foo :bar :baz] :quux))) in the playground: https://clavascript.github.io/clavascript/

I like it

and:

(def x {:a 1})

(assoc-in! x [:b :c] 2)

(js/alert (pr-str x))

beautiful ;)

so nice!

are transducers going to be a thing in clavascript?

gosh, and sequences vs arrays

well, lists vs arrays

I think so, @lilactown just made an excellent writeup about iterators and transducers

oh cool

https://github.com/clavascript/clavascript/issues/22 for people following along

wow, this is much trickier than I thought

great writeup @lilactown

this might not be the first time I've thought about seqs and transducers in JS... 😅

https://twitter.com/borkdude/status/1559277968513503233

how do I add a new core var? I tried adding first and rest to resources/clava/core.edn but I'm still seeing

$ ./node_cli.js --show -e '(first [1 2 3])'
first([1, 2, 3]);

file:///Users/lilactown/Code/clavascript/.tmpSIEsgl/clava.mjs:1
first([1, 2, 3]);
^

ReferenceError: first is not defined
    at file:///Users/lilactown/Code/clavascript/.tmpSIEsgl/

@lilactown Resources is generated when you do bb dev

I've tried restarting too

And that .edn file is then pulled in via a macro. But macros are cached in CLJS, so maybe we need to "touch" the macro file as well

Try rm -rf .shadow-cljs

I guess we could also just generate .cljs file with that data

removing the .shadow-cljs dir fixed it

thanks!

I'm thinking of opening PRs for some of these seq ops

should I create separate issues outside of my writeup?

I can also wait for people to chime in if more time in the hammock is wanted

if you want to get them merged in faster, I recommend smaller PRs, because it's easier to reason about small changes

I think the first/rest thing is a no-brainer

@lilactown Thanks for the PR. With eq you can also use a nested Clojure structure since it's thrown through clj->js in the eq function. You don't have to change this, just a FYI

ah cool

Btw, one of the reasons I think clavascript is cool, is that you get a more direct way to write JS and with bun focussing on performance, writing FFI-ish stuff for that will be very interesting and fast

E.g. this: https://github.com/babashka/nbb/blob/main/examples/bun/ffi.cljs

oooooh

something I noticed in both ramda and transducers-js is that they check for whether Symbol is defined or not

I wonder which platforms this is still true for

https://github.com/cognitect-labs/transducers-js/blob/master/src/com/cognitect/transducers.js#L44

ahhh, IE

our old friend

well, we won't support that one, so we're cool :)

G'night enclava!

night!