cgrand / macrovich

A set of three macros to ease writing `*.cljc` supporting Clojure, Clojurescript and self-hosted Clojurescript.
Eclipse Public License 1.0
163 stars 6 forks source link

Can't work out how to require node modules when using case macro #8

Open alza-bitz opened 5 years ago

alza-bitz commented 5 years ago

Hi there,

I have a situation where I would like to provide a single named macro for Clojure and ClojureScript, but there needs to be a different implementation for each.

I believe that the case macro of Macrovich is what I need, however in my case I need the ClojureScript branch to produce code that uses a NodeJS built-in module, and after trying a few different ways I can't get it to work.

Approach 1 code:

(ns issue-1
  (:require
   #?(:cljs ["console" :as console]) 
   [net.cgrand.macrovich :as m #?@(:cljs [:include-macros true])]))

(defmacro notworking
  []
  `(m/case
    :cljs
     (.log console "Hello, world!")))

#?(:cljs 
   (defn working
     []
     (.log console "Hello, world!")))

Approach 1 results:

(require '[issue-1 :as issue-1 #?@(:cljs [:include-macros true])] :reload-all)

(issue-1/working)
; Hello, world!
; nil

(macroexpand '(issue-1/notworking))
; (. issue-1/console log "Hello, world!")

(issue-1/notworking)
; WARNING: Use of undeclared Var issue-1/console at line 1 <cljs repl>
; Execution error (Error) at (<cljs repl>:1).
; Cannot read property 'log' of undefined

Approach 2 code:

(ns issue-2
  (:require
   [net.cgrand.macrovich :as m #?@(:cljs [:include-macros true])]))

(defmacro notworking
  []
  `(m/case
    :cljs
     (require '["console" :as console])
     (.log console "Hello, world!")))

Approach 2 results:

(require '[issue-2 :as issue-2 #?@(:cljs [:include-macros true])] :reload-all)

(macroexpand '(issue-2/notworking))
; Unexpected error (IllegalArgumentException) macroexpanding cljs.core/macroexpand at (<cljs repl>:1:1).
; No value supplied for key: (.log issue-2/console "Hello, world!")

Please can you help/advise me?

Thanks!

alza-bitz commented 5 years ago

Just a quick follow-up: I managed to work around the problem by redefining the macro..

Approach 3:

(defmacro macro-working
  []
  `(m/case
    :cljs
     (.log js/console "Hello, world!")))

Approach 3 results:

(macroexpand '(issue-1/macro-working))
; (. js/console log "Hello, world!")

(issue-1/macro-working)
; Hello, world!
; nil

However, I'd still like to understand why approaches 1 and 2 didn't work 🙂

borkdude commented 5 years ago

@alzadude Could you try js/console instead of console in the first approach to begin with? Or are you using some kind of console library?

alza-bitz commented 5 years ago

@alzadude Could you try js/console instead of console in the first approach to begin with? Or are you using some kind of console library?

Hi there, yes using the js/console form seems to get round the problem. This macro is working:

(defmacro console-macro-working
  []
  `(m/case
    :cljs
     (.log js/console "Hello, world!")))

I actually only used console in the issue because I wanted the simplest case that reproduced the issue. For the project I'm working on, I actually need the fs module instead. Luckily, using that in place of console seems to work, for example this other macro using fs is working:

(defmacro fs-macro-working
  []
  `(m/case
    :cljs
     (.openSync js/fs "file.txt" "r")))

However, I'm not quite done, because when I call this macro from a test run, I am getting:

#object[ReferenceError ReferenceError: fs is not defined]

I currently have no idea why this is working at the repl, but not in tests. I'll continue investigating..

alza-bitz commented 5 years ago

Ok, progress update..

Since I don't understand why js/fs doesn't work in tests (and also when ClojureScript is compiled), even though it works at a PiggieBack repl, I went back to trying a variant of approach 2, and I came up with something that works (both at the repl and in tests).

Approach 4:

(defmacro fs-macro-working
  []
  `(m/case
    :cljs
     (let [fs# (js/require "fs")]
       (.openSync fs# "file.txt" "r"))))

The reason approach 2 failed with No value supplied for key, is because the case macro expects a single form to be associated with each of :clj and :cljs, and in approach 2 I had two forms instead of one. This wasn't so obvious from the README, and so it tripped me up for a while..