Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable calling clojure.tools.namespace.repl/set-refresh-dirs as part of setup. #59

Closed
grzm opened this issue Aug 27, 2016 · 7 comments
Closed

Comments

@grzm
Copy link

grzm commented Aug 27, 2016

.cljc files in /resources/ (such as those copied there during figwheel compilation) are picked up by default during clojure.tools.namespace.repl/refresh, which can stomp on code loaded into memory by test-refresh. By exposing clojure.tools.namespace.repl/set-refresh-dirs in some way as part of the test-refresh run, we can avoid this from happening.

For more detail, see a blog post I wrote detailing how I worked around the same issue with the Reloaded workflow

@grzm
Copy link
Author

grzm commented Aug 27, 2016

I believe this only needs to be called once, not on every test run. Perhaps have a "setup-fn" that's specified in project.clj and called in leiningen.test-refresh/test-refresh?

@jakemcc
Copy link
Owner

jakemcc commented Aug 28, 2016

Interesting problem.

Allowing a generic setup-fn would allow lein-test-refresh users to do whatever they want (depending on implementationeither prior to every test run or just once before running any tests). That is an interesting approach as it would let people do whatever they want and I think would enable some neat things. If implemented for solving this problem it does put quite a burden on user of lein-test-refresh. I think the function would need to be called in the com.jakemccrary.test-refresh/monitor-project as that is when code is being evaluated within the context of the project.

Another approach would be to add a :refresh-dirs option that is similar to the :watch-dirs option. This feels a lot easier for users.

I won't be able to give a shot at either solution until sometime later this week. I'd most likely accept a PR for :refresh-dirs immediately. I'd need to think about the setup-fn idea a bit before getting that feature into the codebase.

@jakemcc
Copy link
Owner

jakemcc commented Aug 29, 2016

Actually, was thinking about some weirdness I've been seeing in one of my own projects and wondering if its because of this problem. Going to build out the first option today.

@jakemcc
Copy link
Owner

jakemcc commented Aug 29, 2016

Released 0.16.1-SNAPSHOT with a :refresh-dirs option. Check the sample.project.clj for what it looks like.

Sort of wondering if :watch-dirs and :refresh-dirs should be collapsed into a single option. I'm having a difficult time coming up with a scenario in which you would set one and not the other.

@grzm
Copy link
Author

grzm commented Aug 29, 2016

Thanks! From my initial testing, setting :refresh-dirs with 0.16.1-SNAPSHOT works for me!

I don't know enough about how Figwheel does its compilation, but I wonder if Figwheel could be convinced to place its compilation someplace other than where the final JavaScript output ends up. Of course that's an issue for a different project :)

Thanks again for your quick response!

@jakemcc
Copy link
Owner

jakemcc commented Sep 1, 2016

Just released a non-snapshot release, 0.17.0, with this feature.

@jakemcc jakemcc closed this as completed Sep 1, 2016
@grzm
Copy link
Author

grzm commented Sep 1, 2016

Thanks!

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

No branches or pull requests

2 participants