metosin / sieppari

Small, fast, and complete interceptor library for Clojure/Script
Eclipse Public License 2.0
207 stars 21 forks source link

Port to ClojureScript #10

Closed arichiardi closed 5 years ago

arichiardi commented 6 years ago

This patch ports the code to ClojureScript - self-host included. The only big difference is that we cannot run blocking code in JavaScript. Therefore the 2-arity version of sieppari.core/execute cannot be included.


Unfortunately, I kind of got stuck with the tests. I did not expect that testit framework :smile:

I created test/cljs and a test/clj folder in order to separate the two and kind of convert to CLJS only the things I need. Manifold will never work obviously.

Lumo and figwheel are used for testing cljs and I still have a couple of tests to port.


To try it out in lumo:

$ lumo -c src:examples
Lumo 1.8.0
ClojureScript 1.9.946
Node.js v9.2.0
 Docs: (doc function-name-here)
        (find-doc "part-of-name-here")
         Source: (source function-name-here)
          Exit: Control+D or :cljs/quit or exit

cljs.user=> (require 'example.simple)
{:y 42}
nil
arichiardi commented 6 years ago

I have to be that guy, I know it is not the first and it won't be the last.

I have to be the guy that thought it was the code's fault, not his own...the guy who thought the machine was wrong, but no. I was wrong of course.

Today, thankfully, I realized that my tests were completely wrong and so was my assertion above.

It is all good, I have added tests for promesa and native promises. Both work fine but for some obscure reason cannot be mixed. It might be that promesa overrides the protocols set on js/Promise.

Therefore promesa is tested by figwheel and native by lumo (which cannot run promesa anyway).

jarppe commented 6 years ago

I'm very sorry that I have not been able to give you the feedback you clearly deserve, sorry for that. I'll look into this during the weekend.

arichiardi commented 6 years ago

This is now officially out of WIP, ported the last couple of things :smile:

As a note, I converted some clj tests and now this is the situation:

test
├── clj
│   └── sieppari
│       ├── async
│       │   └── manifold_test.clj
│       ├── core_async_test.clj
│       ├── core_execute_test.clj
│       ├── core_test.clj
│       ├── manifold_test.clj
│       └── promesa_test.clj
├── cljc
│   └── sieppari
│       ├── async
│       │   ├── core_async_test.cljc
│       │   └── promesa_test.cljc
│       ├── context_test.cljc
│       ├── interceptor_test.cljc
│       └── queue_test.cljc
└── cljs
    └── sieppari
        ├── core_async_test.cljs
        ├── core_execute_test.cljs
        ├── native_promise_test.cljs
        ├── promesa_test.cljs
        └── runner.cljs

No rush on this, we will slowly start to port some of our code to it, I am just glad it is now Cljs compatible ;)

arichiardi commented 5 years ago

We are now using this guy in our node apps...it would be awesome if we could get a review :smile:

miikka commented 5 years ago

@arichiardi In case Jarppe is still busy, I can review this. I'll chat with him today.

arichiardi commented 5 years ago

@miikka the deps.edn was my choice because I guess is the way forward for Clojure projects and because I am familiar with it as well. It is also more convenient to setup for figwheel testing. No big big reason really but convenience and familiarity and copy and paste :smile:

I tried to call eftest from that but it did not seem to work. Some test was hanging: see here.

So I decided not to do all this work in one PR, leave the two tools, but prepare things for coming PRs.

One thing I could try is to use https://github.com/RickMoynihan/lein-tools-deps, which is already working in our projects so that you will only leave one place to manage dependencies in.

What do you think?

miikka commented 5 years ago

I'll just merge this now. I think that eventually the build tooling needs to be consolidated, but Sieppari is in version 0.0.0-alpha6 anyway, so I guess it is fine to have many build tools.