search

reagent-project / reagent-template

A Leiningen template for projects using Reagent.
MIT License
394 stars 55 forks source link

Stop using profile merging for :cljsbuild key. Just use different builds. #119

Closed bhauman closed 8 years ago

bhauman commented 8 years ago

There are several reasons for this.

One reason is :output-dir contamination. Having both compiles go to the same output directory can lead to confusing compilation states when moving from dev to prod.

My main reason is that this is where people look for an example of how to set up a ClojureScript project and (start-figwheel!) can't do profile merging and it ends up confusing a lot of people. There is are several reasons for figwheel not to do profile merging, but the biggest one is so that we aren't pulling all of Leiningen into the process that is running the compiler and possibly the dev server. Leiningen is a big dependency that changes slowly and this can lead to all sorts of dependency tree conflicts.

Another reason is that profile merging is just not needed.

The configuration for :figwheel and :cljsbuild are "just data". They do not change the runtime environment the way that something like :dependencies or something else that modifies the classpath.

And since :cljsbuild lets you have as many builds as you need, there is no need to use profile merging to get the behavior you want.

So it would be extremely helpful if we could change this to use multiple builds in cljsbuild.

jamesmintram commented 8 years ago

+1 - I am one of the people who ended up getting confused :)

yogthos commented 8 years ago

That sounds reasonable to me, @bhauman would you be up for making a pr with the fix? :)

plexus commented 8 years ago

I addressed the same issue in Chestnut, in case you're looking for an example. YMMV of course :)

Also feedback welcome, there's probably still plenty of things Chestnut could be doing better.

https://github.com/plexus/chestnut/pull/173

yogthos commented 8 years ago

@plexus yeah that looks great, I definitely agree with the change

yogthos commented 8 years ago

@bhauman @plexus does this look good to you guys? https://github.com/reagent-project/reagent-template/commit/6fd8490400f26ee1de13e3b48e57ebfd33965119

bhauman commented 8 years ago

Please put : figwheel at the top level as well. Other than that it looks great!

yogthos commented 8 years ago

Ok perfect, just released to Clojars with the fix.

bhauman commented 8 years ago

Sweet!!!

yogthos commented 8 years ago

also updated the Luminus template to match :)

bhauman commented 8 years ago

Sweeter!