Skip to content

Genericized (was test.check specific) compiler changes for downstream macro/ns resolution#805

Open
willcohen wants to merge 7 commits intosquint-cljs:mainfrom
willcohen:test-check
Open

Genericized (was test.check specific) compiler changes for downstream macro/ns resolution#805
willcohen wants to merge 7 commits intosquint-cljs:mainfrom
willcohen:test-check

Conversation

@willcohen
Copy link
Copy Markdown
Contributor

@willcohen willcohen commented Apr 17, 2026

Reopening as followup to #803, which autoclosed when #802 merged. Needed for squint-cljs/cherry#184, which I similarly need to rebase and re-open based on this.

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

@willcohen willcohen changed the title Recognize test.check macros and namespaces in cherry-targeted compila… Recognize test.check macros and namespaces in cherry-targeted compilation Apr 17, 2026
@willcohen willcohen marked this pull request as draft April 17, 2026 18:53
@willcohen willcohen marked this pull request as draft April 17, 2026 18:53
@willcohen
Copy link
Copy Markdown
Contributor Author

Eep. Stuff still changing underneath. This can happen later.

@borkdude
Copy link
Copy Markdown
Member

Hehe sorry. I wasn't done with cljs.test but it's published now and no changes planned.

@willcohen willcohen force-pushed the test-check branch 2 times, most recently from 358ee4f to 3293973 Compare April 17, 2026 19:38
@willcohen willcohen marked this pull request as ready for review April 17, 2026 20:48
@willcohen
Copy link
Copy Markdown
Contributor Author

@borkdude this is now ready — took a little finagling after cljs.test settled. One more small proposed change to macros. I needed this for test.check because defspec bodies carry (prop/for-all …) aliased (not :refer'd), so without this the macro never fired: the compiled JS emitted a raw prop.for_all(…) runtime call, which was causing undefined errors.

diff --git a/src/squint/compiler_common.cljc b/src/squint/compiler_common.cljc
index a57e5959..3afcd824 100644
--- a/src/squint/compiler_common.cljc
+++ b/src/squint/compiler_common.cljc
@@ -809,6 +809,9 @@
                      as
                      (update-in [current :aliases] (fn [aliases]
                                                      ((fnil assoc {}) aliases as libname)))
+                     (and as (symbol? original-libname))
+                     (update-in [current :macro-aliases]
+                                (fn [aliases] (merge {as original-libname} aliases)))
                      (symbol? original-libname)
                      (update-in [current :aliases] (fn [aliases]
                                                      ((fnil assoc {}) aliases

@willcohen
Copy link
Copy Markdown
Contributor Author

Rebased to sit on top of latest changes to squint. squint-cljs/cherry#186 updated as well.

@borkdude
Copy link
Copy Markdown
Member

I'm not yet sure about this approach. clojure.test is a built-in namespace but clojure.test.check is an external dependency in ClojureScript so I don't think this change belongs in squint.

@willcohen
Copy link
Copy Markdown
Contributor Author

Makes sense. I'll give it a shot.

@willcohen willcohen changed the title Recognize test.check macros and namespaces in cherry-targeted compilation Genericized (was test.check specific) compiler changes for downstream macro/ns resolution Apr 27, 2026
@willcohen
Copy link
Copy Markdown
Contributor Author

Here's a lighter touch approach to squint. I dropped the test.check specific stuff here, and cherry now handles that via env.

Three new commits:

First: "Populate :macro-aliases from :require :as so plain require finds built-in macros". Same content as before, rebased onto current main.
Second (new): "Make :cherry target of resolve-ns honor :resolve-ns env hook for symmetry with :squint". Cherry was already passing :resolve-ns from cli.cljs, just unused on the :cherry path.
Third (also new): "Filter :refer'd macros via env-supplied :built-in-macro-nss". Adds an inline env hook in the :refer filter so downstream targets register their own macro names without squint knowing about them.

@willcohen
Copy link
Copy Markdown
Contributor Author

To better decouple this from the various cherry PRs that depend on it, I've added red/green tests preceding each of the three commits, which should allow this to be self-contained. @borkdude

@willcohen
Copy link
Copy Markdown
Contributor Author

Merged main in so this now applies cleanly after #816.

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