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

Support for code-splitting after cljs 1.9.854? #613

Closed magnars closed 7 years ago

magnars commented 7 years ago

I'm trying to use the new code-splitting feature via :modules released in clojurescript 1.9.854. Is this something figwheel supports? Or should support?

Ref. https://clojurescript.org/guides/code-splitting

bhauman commented 7 years ago

Figwheel should support it. And will support it. I'll have to look into it but I'm sure you can make it work manually by creating your client initialization code and similar to the figwheel connect scripts that figwheel generates. You would have to remove the figwheel config from the build config and place it in the connect script manually

On Sep 26, 2017, at 6:37 AM, Magnar Sveen notifications@github.com wrote:

I'm trying to use the new code-splitting feature via :modules released in clojurescript 1.9.854. Is this something figwheel supports? Or should support?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

bhauman commented 7 years ago

I've been hammocking this for a couple of days and I have found a really good solution to make this automatic. I'm working on it right now.

magnars commented 7 years ago

Wow, that sounds great. 👌🙂 ons. 27. sep. 2017 kl. 17.11 skrev Bruce Hauman notifications@github.com:

I've been hammocking this for a couple of days and I have found a really good solution to make this automatic. I'm working on it right now.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bhauman/lein-figwheel/issues/613#issuecomment-332554437, or mute the thread https://github.com/notifications/unsubscribe-auth/AAQOOWOVwyFR7Wc4Y4rQdTxJk56lVGNbks5smmWrgaJpZM4PkCL3 .

pkpkpk commented 7 years ago

Is it at all possible to respect modules defined in checkouts?

bhauman commented 7 years ago

@magnars give 0.5.15-SNAPSHOT a try with a modules based project It should just work.

magnars commented 7 years ago

Hi Bruce! Thanks for looking into this. I have now tried this snapshot, with some good and some bad news. :)

The good news is that modules now work. They are compiled by figwheel, and changes are detected. :+1:

The bad news is that reloading is blocked:

not required: ("resources/public/js/compiled/out/bar/core.js")

There's a good reason for this: the bar.core namespace is in fact not required - it is resolved. See https://clojurescript.org/guides/code-splitting:

"Notice that we use resolve. This is because we cannot directly call something we never required. If we tried to do this without resolve the ClojureScript compiler would emit an undeclared var warning during compilation."

I've made a small repro-repo here: https://github.com/magnars/bug-examples/tree/lein-figwheel-613

magnars commented 7 years ago

There are a few small niggles as well. Just thought I'd mention them:

These aren't blockers to using modules with figwheel at all, tho. The reloading vs require/resolve is. At least if you're planning on using modules to delay loading of code like this. :)

bhauman commented 7 years ago

OK leaving aside the warnings and such there is very good reason for this behavior.

You do need to "load" your module before you can start reloading it. Once the namespace is loaded it will reload just fine.

Figwheel never reloads files that aren't in the dependency graph by default. Once you load your module then it should be in the dependency chain and reload just fine.

And that is what ^:figwheel-load meta data on the namespace if for. You just place it there to force the loading of a file the first time and then you can remove it.

I'm going to check that this all works as expected right now.

bhauman commented 7 years ago

OK darn looks like I'm wrong about this ...

bhauman commented 7 years ago

You are correct in that it doesn't reload even if it has been loaded.

The ^:figwheel-load meta data does work however.

I'm going to check and see how to detect if something is a loaded module to fix the above bug.

bhauman commented 7 years ago

Alright, I believe it now behaves correctly.

https://github.com/bhauman/lein-figwheel/commit/0ab52dca9289549eea31f11ef633d7f41c6033a2

Once the NS is loaded it will start being reloaded.

@magnars I just re-deployed a SNAPSHOT with this fix, at your convenience, please let me know if its working for you.

bhauman commented 7 years ago

And I should have just fixed all the warning and messaging around using :modules. And its all deployed to the SNAPSHOT

magnars commented 7 years ago

I'm not seeing any of the changes. I did a lein -U figwheel, and seeing no snapshots downloaded, I removed lein-figwheel from my .m2. Then it downloaded these:

Retrieving lein-figwheel/lein-figwheel/0.5.15-SNAPSHOT/lein-figwheel-0.5.15-20171003.212007-1.pom from clojars
Retrieving lein-figwheel/lein-figwheel/0.5.15-SNAPSHOT/lein-figwheel-0.5.15-20171003.212007-1.jar from clojars

Is that the updated version?

bhauman commented 7 years ago

You shouldn't have to do this, but you may have to remove figwheel and figwheel-sidecar from ".m2" if you are not getting the updates.

On Wed, Oct 4, 2017 at 1:29 PM, Magnar Sveen notifications@github.com wrote:

I'm not seeing any of the changes. I did a lein -U figwheel, and seeing no snapshots downloaded, I removed lein-figwheel from my .m2. Then it downloaded these:

Retrieving lein-figwheel/lein-figwheel/0.5.15-SNAPSHOT/lein-figwheel-0.5.15-20171003.212007-1.pom from clojars Retrieving lein-figwheel/lein-figwheel/0.5.15-SNAPSHOT/lein-figwheel-0.5.15-20171003.212007-1.jar from clojars

Is that the updated version?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bhauman/lein-figwheel/issues/613#issuecomment-334229967, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAKQDUylg12nx-6cCKasAuRm-alNDZjks5so8BxgaJpZM4PkCL3 .

magnars commented 7 years ago

That did the trick. Everything seems to be working perfectly now. 👍😄