clj-commons / virgil

Recompile Java code without restarting the REPL
306 stars 21 forks source link

Integration with clojure.tools.namespace.repl workflow #22

Open aiba opened 6 years ago

aiba commented 6 years ago

I write performance-sensitive clojure code, sometimes dropping into java, and virgil has been great for this. Thank you!

But frankly I don't want virgil to recompile on file saves. Would you want your clojure namespaces to recompile on save? For clojure I use clojure.tools.namespace.repl/refresh to manually trigger a refresh of necessary namespaces. I'd love for virgil to integrate with this.

The current story is to call virgil.compile/compile-all-java and then clojure.tools.namespace.repl/refresh-all as suggested by a user in #20. But recompiling all clojure namespaces is unecessarily slow (~30 seconds in my primary project). You really only need to recompile clojure namespaces that depend on changed java classes, and their dependents.

I think we can make this happen, and here's how it could work:

With this, anyone who wants to integrate java files into their refresh workflow could just call (virgil.repl/refresh) before calling (clojure.tools.namespace.repl/refresh).

I made a proof of concept and have been using it in my project successfully.

If you like the concept, I'd be happy to clean this up and submit it as a PR. This could just become a new namespace, virgil.repl. Virgil already depends on clojure.tools.namespace.

What do you think?

schmee commented 6 years ago

@aiba Please make a pull request! I would love to try it out.

ztellman commented 6 years ago

I actually tried to do a minimal namespace refresh in earlier versions of Virgil, and it led to some problems (unreloaded namespaces that depend on protocols defined in the reloaded namespaces, for instance), which is why I ended up using the nuclear option.

However, I'm happy to create an alternate code path that has some caveats attached, if it doesn't touch the existing automatic reload functionality.

aiba commented 6 years ago

actually tried to do a minimal namespace refresh in earlier versions of Virgil, and it led to some problems (unreloaded namespaces that depend on protocols defined in the reloaded namespaces, for instance), which is why I ended up using the nuclear option.

I think the pasted gist above is handling this case by also reloading transitive dependents as well as immediate dependents of recompiled java classes. It's just leveraging clojure.tools.namespace to do it.

However, I'm happy to create an alternate code path that has some caveats attached, if it doesn't touch the existing automatic reload functionality.

Great to hear! I have learned some things using the above gist on my own project for the past few weeks. I will polish it up and submit a PR. It will not modify any existing virgil code.