bhauman / lein-figwheel

Figwheel builds your ClojureScript code and hot loads it into the browser as you are coding!
Eclipse Public License 1.0
2.88k stars 210 forks source link

Client configs with cljs 1.10 break #665

Open yannvanhalewyn opened 6 years ago

yannvanhalewyn commented 6 years ago

It looks like cljs 1.10 breaks the ability to have multiple figwheel builds, even across projects. It seems that cljs 1.10 caches compiled dependencies, and after building figwheel the first time the config for that specific build sticks. This read-config macro: https://github.com/bhauman/lein-figwheel/blob/master/support/src/figwheel/connect.cljs#L9 effectively only runs the first time any figwheel resources are compiled.

Reproduction:

Run both builds, and checkout both resources/public/js/compiled/{out1|out2}/figwheel/connect.js and confirm the var config = statement is always the same (even when previously built in another project). They now either both have the on-jsload hook, or neither.

Consequences

No need to elaborate, but issues with js-load hooks not being configured, repl's not connecting because the build id doesn't match, ...

I'm not entirely sure if I should post this here or at cljs, what do you think @bhauman?

yannvanhalewyn commented 6 years ago

Not all too familiar with the figwheel codebase or the closure compiler, but a solution might be to pass in the config to connect.start in the generated output.js:

document.write("<script>figwheel.connect.start(config);</script>");

if that's achievable.

bhauman commented 6 years ago

Does :aot-cache false fix it?

You'll probably have to do :validate-config false as well.

bhauman commented 6 years ago

cc/ @swannodette

bhauman commented 6 years ago

cc/ @darwin this will probably be a problem for dirac configuration as well?

yannvanhalewyn commented 6 years ago

Not sure where the :aot-cache option goes, could you specify?

bhauman commented 6 years ago

it would go in the :compiler options for the build. and :validate-config false would be under the top level figwheel config.

yannvanhalewyn commented 6 years ago

That's what I thought but was missing :validate-config. I'll try it.

bhauman commented 6 years ago

Don't forget to delete all the assets even the cache.

yannvanhalewyn commented 6 years ago

I didn't :)

Confirmed, clearing ~/.cljs/.aot_cache, lein clean and running with those options works.

yannvanhalewyn commented 6 years ago

It consistently fails without them on a clean project, so I guess a more durable solution is needed. I'll gladly help out but I'm struggling a bit to find where changes can be made in the codebase to submit a PR.

bhauman commented 6 years ago

AOT cache was introduced to help with command line start up times. I'm thinking the smart solution for now is to default to :aot-cache false for figwheel.

bhauman commented 6 years ago

It will also, no doubt, help prevent folks from experiencing other weird behavior when the cache delivers stale code.

yannvanhalewyn commented 6 years ago

That definitely would work. That or moving configuration to figwheel.preload.

bhauman commented 6 years ago

How would that fix it?

bhauman commented 6 years ago

The preload would get cached just as well correct?

yannvanhalewyn commented 6 years ago

Thought it might run at compile time, I don't see the bigger picture here though. :aot-cache false as default seems like the easier and better idea indeed.

bhauman commented 6 years ago

So actually :aot-cache false should work even if you don't clear the cache though, is that correct?

yannvanhalewyn commented 6 years ago

I guess, it seems like it shouldn't go look for any cache files also.

yannvanhalewyn commented 6 years ago

I'll try it real quick

bhauman commented 6 years ago

Thanks!

bhauman commented 6 years ago

I'll have a snapshot deployed in a jiffy

yannvanhalewyn commented 6 years ago

Allright, confirmed also that :aot-cache false works when there already is a cache!

I'll have a snapshot deployed in a jiffy

Awesome 😄

bhauman commented 6 years ago

Great!

bhauman commented 6 years ago

Looks like :aot-cache false will be the default in the compiler.

bhauman commented 6 years ago

So I'm not going to deploy a snapshot that sets :aot-cache false. I am going to deploy a snapshot that accepts all the new config options.

yannvanhalewyn commented 6 years ago

I saw the commit, all will be good then. Thanks so much for the quick responses and actions @bhauman!