Support macros that expand to require forms

Description

Support this kind of code:

(ns foo.bar
(:require-macros [foo.baz :refer [macro-that-expands-to-require]]))
(macro-that-expands-to-require)

Note that we only plan to support this specific case, i.e. it has to come right after the ns form.

This did not work yet because in parse-ns the :ns op did not recur, so we never looked at the second form.

The recur is easy to fix, but after that analyzing the second form (the macro invocation) fails, because the macros are not yet loaded (parse-ns calls analyze but with *load-macros* false).

Environment

None

Activity

Show:
Thomas Heller
August 23, 2020, 7:12 PM

PS: Given that a function can modify the ns data freely it may also remove certain requires which the macro variant cannot do. This would be sometimes desirable to remove or add development requires or replace them with something more amendable to DCE.

Arne Brasseur
August 24, 2020, 8:46 AM

From my side the use case driving this is injecting test namespaces into a build in kaocha-cljs2 and chui, in a way that is easily accessible to end users regardless of the build tool they use (cljs.main, figwheel, boot, etc.). Because of this requirement it should be something that works on the language level (like this proposal, add a require+call a macro), or Thomas’s proposal (ns metadata), or it could work on the compiler-opts level, e.g. by having build hooks that people reference in their compiler options.

So I think so far we’ve identified these possible approaches

  • have explicit test scanning functionality in every build tool, the way shadow-cljs has. Not ideal because of the proliferation of slightly different implementations/configurations, but could work for me.

  • have test scanning functionality in ClojureScript. This was proposed but rejected as being outside the scope of ClojureScript’s responsibilities.

  • require via macro, i.e. this proposal. It seems this was actually assumed to already work, and we’re really only adding something that you can already do in Clojure. That said it’s an imperfect reproduction since it would only work immediately following the ns form. It would be a special case feature for power users that are aware of its limitations.

  • hook functions to hook into the ns analysis via ns metadata, i.e. Thomas’s proposal. Would work for me too.

  • build hooks a la shadow-cljs, so one can inject extra forms to be compiled into the build. A library can provide the hook function that end users simply add to their compiler opts. Would also work for me.

In the end any of these would work for me, and I’m happy to help with implementing any of these if they get the explicit ok.

Thomas Heller
August 24, 2020, 11:34 AM

As far as testing is concerned I think the best option is to keep it in the build config and out of the code completely. Obviously I’m biased since that is how it works in shadow-cljs but the whole point of that is that the user does not have to write a “test runner” namespace at all in the first place. Of course that is still possible but usually not required.

Namespaces such as kinda look like they should be build config or even command line arguments instead and use a generic reuasable “runner”. I think the :target-fn addition already enables that?

Again I keep coming back to this that the only issue I have with this is solving it via macros. In Clojure we have the luxury of just calling require at runtime dynamically. Doing this via macros adds all sorts of footguns and using them to drive the build seems completely backwards to me.

Thomas Heller
August 24, 2020, 11:59 AM

Back to the other issue of slightly more dynamic require for CLJS. One rather common issue is that some namespaces should only be included in development builds but removed for release builds. One common library that has this issue is the re-frame-10x or the fulcro guardrails. They both add macros that add heavy instrumentation and have a bunch of requires. While the instrumentation can be easily disabled and skipped in the macros the initial require of those namespaces is not so easily removed and bloat the final output quite a bit with unused code which isn’t removable by DCE either.

So the common instructions include swapping out the dependency on the classpath [1] depending on what you are building. In shadow-cljs I added the :ns-aliases option [2] so that you don’t have to mess with the classpath but instead can just replace a single namespace with a stub. Both get the job done of course but I think messing with the classpath is rather unfriendly to the user.

This would be another candidate where having a slightly more dynamic require would be desirable which also cannot strictly be expressed via build tooling alone. You need the actual namespaces to provide to required code, aliased or not. I still think this is better expressed via build config though. Likely that will be used to configure whatever the “dynamic require” is supposed to do anyways, regardless if its a macro so :ns-mod.

 

[1]

[2]

Arne Brasseur
September 12, 2020, 12:36 PM

I added CLJS-3276-3.patch which adds a test.

Assignee

David Nolen

Reporter

Arne Brasseur

Labels

None

Approval

Screened

Patch

None

Priority

Major
Configure