Skip to content

Fix :macros support in cherry#160

Closed
willcohen wants to merge 1 commit intosquint-cljs:mainfrom
willcohen:macros
Closed

Fix :macros support in cherry#160
willcohen wants to merge 1 commit intosquint-cljs:mainfrom
willcohen:macros

Conversation

@willcohen
Copy link
Copy Markdown
Contributor

Please answer the following questions and leave the below in as part of your PR.

This PR addresses this issue by adding symbolize-macros-map (converts string keys to symbols) and build-macro-refers (auto-populates :refers maps for unqualified calls). Users can now pass macro functions to compileString via :macros option, supporting both qualified (my.ns/my-macro x) and unqualified (my-macro x) calls. CLI :require-macros still unsupported due to path resolution issues.

(is (not (str/includes? result "my_macro"))
"Should not contain function call"))))

(deftest unqualified-macro-test
Copy link
Copy Markdown
Member

@borkdude borkdude Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to make macros available unqualified I'd add them to the cljs.core or clojure.core namespace in :macros
Don't make them available unquaiified by default.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another approach is to make a compiler state and compile a (:require-macros [foo.bar :refer [x y z]) and use the same compiler state over and over again

(deftest macro-across-ns-change-test
(testing "macro works after (ns ...) form"
(let [code "(ns user)\n(my-macro 1)\n(ns other)\n(my-macro 2)"
result (cherry/compile-string code #js {:macros #js {"my.ns" #js {"my-macro" my-macro-fn}}})]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't work at all but is related to my remarks about unqualified macros

(deftest qualified-macro-test
(testing "namespace-qualified macro call"
(let [code "(my.ns/my-macro 42)"
result (cherry/compile-string code #js {:macros #js {"my.ns" #js {"my-macro" my-macro-fn}}})]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default compile-string function doesn't support JS objects I believe?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, it does. It would probably be cleaner to split this out like compileStringEx

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

Successfully merging this pull request may close these issues.

2 participants