Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

error when calling a macro which is referring to a js library symbol #309

Closed
ikappaki opened this issue Feb 23, 2023 · 3 comments
Closed

Comments

@ikappaki
Copy link
Contributor

ikappaki commented Feb 23, 2023

version

v1.2.163

platform

any

problem

nbb errors out when calling a macro which is referring to a js library symbol:

ja.call is not a function

repro

  1. Create an nbb project point to src dir
{:paths ["src"]}
  1. Create a src/macros.clj file with a trivial macro calling a node's fs js library fn, and another macors calling a cljs lib macro (the latter is just for comparison). Note how the exists-sync macro fails because the js lib fn it is referring to (existsSync) is not fully qualified, but the test-is macro succeeds for the same
(ns macros
  (:require [cljs.test :refer [is]]
            ["fs" :refer [existsSync] :as filesystem]))


;; Can call a macro referring to a cljs library symbol fine.
;;
(defmacro test-is []
  `(is (= 5 3)))
(test-is)
;; => false 
(macroexpand-1 '(test-is))
;; => (cljs.test/is (clojure.core/= 5 3))

;; (case 1) But cannot call a macro referring to a js library symbol.
;;
(defmacro exists-sync []
  `(existsSync "."))
#_(exists-sync)
;; => ja.call is not a function
(macroexpand-1 '(exists-sync))
;; => ("nbb.internal.fs$macros$existsSync" ".")

;; Thought can call a macro referring to a js whose symbol when is
;; fully qualified fine.
;; 
(defmacro filesystem-exists-sync []
  `(filesystem/existsSync "."))
(filesystem-exists-sync)
;; => true
(macroexpand-1 '(filesystem-exists-sync))
;; => (filesystem/existsSync ".")
  1. Create a src/issue.cljs file that require's the macros namespace. Notice how the macros/test-is macro referring to the unqualified cljs lib symbol can be called fine, but neither the macros/exists-sync macro which is referring to the js library unqualified symbol nor the macros/filesystem-exists-sync macro which is referring to the js library qualified symbol works.
(ns issue
  (:require [macros :as m]))

;; Can call macros referring to cljs libs unqualified symbol fine.
;;
(m/test-is)
;; => false
(macroexpand-1 '(m/test-is))
;; => (cljs.test/is (clojure.core/= 5 3))

;; (case 2) but cannot call macros referring to js libs unqualified symbol.
;; 
#_(m/exists-sync)
;; => => ja.call is not a function
(macroexpand-1 '(m/exists-sync))
;; => ("nbb.internal.fs$macros$existsSync" ".")

;; (case 3) neither can call macros referring to js libs qualified symbol.
;; 
#_(m/filesystem-exists-sync)
;; => Could not resolve symbol: filesystem/existsSync
(macroexpand-1 '(m/filesystem-exists-sync))
;; => (filesystem/existsSync ".")

(comment 
  ;; though if the js library is required with the same namespace
  ;; `:as` alias as it was defined in the macro namespace, then it
  ;; works fine.
  (require '["fs" :as filesystem])
  (m/filesystem-exists-sync)
  ;; => true
  (macroexpand-1 '(m/filesystem-exists-sync))
  ;; => (filesystem/existsSync ".")
  
  ;; 
  )

expected behavior

The expectation here is that the referring js library symbols in macros (e.g. existsSync) work the same as cljs library symbols (e.g. is).

There is a workaround for making this work

  1. Prefix each js library symbol in macros with a namespace alias (e.g. filesystem/readSync)
  2. Require the same namespace alias in the namespaces the macro is used (e.g. (:require ["fs" :as filesystem])

But this requires that the file's author where the macro is used at to know what js library namespace have been used in the macro, and it unnecessarily pollutes the file's :require section with the same aliases.

Thanks

@borkdude
Copy link
Collaborator

What is the behavior in regular CLJS?

@borkdude
Copy link
Collaborator

Fixed in 1.2.165

@ikappaki
Copy link
Contributor Author

Fixed in 1.2.165

Thanks, I've updated the original issue report to show the three individual failing cases in the comments.

I can confirm case 1 and 2 are now working, but case 3 still does not, I'll open another issue for the later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants