thheller / shadow-cljs

ClojureScript compilation made easy
https://github.com/thheller/shadow-cljs
Eclipse Public License 1.0
2.23k stars 173 forks source link

How can I use build hooks to pre-empt a compilation #1145

Closed vemv closed 7 months ago

vemv commented 10 months ago

Context

I'm successfully using build hooks for formatting code each time a compilation is triggered.

However, that formatting itself triggers another compilation.

While that is correct behavior, it also is noisy (in terms of emitted output) and potentially slow (compiling code twice is obviously slower than once)

Request

I'd like to know if it's possible for the build hook to modify the build-state so that the compilation is possibly aborted.

The typical usage would be as follows:

Desired end result:

(defn format!
  {:shadow.build/stage :compile-prepare}
  [state]
  (let [reformatted-files (format-files! ,,,)]
    (cond-> state
      (seq reformatted-files) (assoc :shadow.cljs.compilation/abort true))))

Is this already possible? If not, I figure it would be something easy to implement.

Thanks - V

thheller commented 10 months ago

This is not supported. This is not at all what hooks are meant for and I strongly suggest looking for other solutions. Why do this at all as part of the build cycle? Seems like a bad fit to abuse the shadow-cljs file watcher to trigger a build and then make it do something else instead?

vemv commented 10 months ago

Thanks for having a look!

First of all, "format on save" is an extremely common desire.

I'm not abusing shadow-cljs builds for triggering formatting. I certainly can set up a file watcher myself.

What I'm seeking is optimal behavior: files are only compiled after the final formatting is applied. i.e. I can avoid double compilations, which as mentioned are noisy and slower.

For accomplishing that, by definition, I need some sort of coordination with shadow-cljs. Hooks seem a good medium for achieving that coordination.

thheller commented 10 months ago

Would a "format before save" not make more sense? That is how I handle it in Cursive?

vemv commented 10 months ago

I'm aware that there are many approaches to formatting.

As a tooling author, I'm seeking to offer a workflow that a wide variety of Clojure programmers could use, regardless of their choice of IDE, or how that particular IDE might be configured.

In that regard, "format on save" is a universal inferface that anyone can participate in, and that requires zero knowledge and setup.

thheller commented 10 months ago

You can toggle the autobuild feature off completely via (shadow.cljs.devtools.api/watch-set-autobuild! :the-build false) and then enable it again after the formatting finished? You can also drop the autobuild completely and just always trigger the build via (shadow.cljs.devtools.api/watch-compile! :the-build). That'll account for all changes it detected since the last compile.

Would that work?

thheller commented 10 months ago

There is also a 500ms delay before a compilation starts, after changes were detected. Precisely for cases such as this where other things might happen after save. Maybe that just isn't enough for your case? :fs-watch {:loop-wait 1000} would bump that to 1sec instead.

vemv commented 10 months ago

You can toggle the autobuild feature off completely via (shadow.cljs.devtools.api/watch-set-autobuild! :the-build false) and then enable it again after the formatting finished?

I tried this and in appears to leave the build stuck in this state:

[:app] Compiling ..

Maybe that just isn't enough for your case?

That wouldn't be a deterministic fix, and also would make users pay a fixed performance penalty. People typically enjoy fast hot-reloads.

Would you generously take a look at the original proposal? As I see it, it is aligned with the design pattern that is documented in https://shadow-cljs.github.io/docs/UsersGuide.html#build-hooks:

They will receive the build-state (a clojure map with all the current build data) as their first argument and must return this build-state modified or unmodified.

I'm fairly certain that taking a state object, inspecting it for a :shadow.cljs.compilation/abort key and dropping the current workload if found is something that is simple and easy. i.e. refraining from doing work is simple.

I could totally see how a request for e.g. an exotic feature or intrincate logic could be something reasonable to dismiss, but hopefully that's not the case here.

thheller commented 10 months ago

No, I will not be considering solving this via :build-hooks. Implementing it is trivial, but it just feels fundamentally broken as a concept. The decision whether to build or not is not to be made when the build already started. There is already some "progress" happening in the build state before it gets to any hooks, so I also don't want to potentially having to "rollback" those changes.

That being said I'm willing to consider alternate solutions.

So, lets start with the watch-set-autobuild! I suggested. It is working fine for me locally and doesn't get stuck. If you provide the steps to reproduce I can take a closer look.

It would help immensely if your entire setup is somewhat reproducible, so I can see what stuff is actually doing and when. Also what kind of capabilities it may have.