tolitius / mount

managing Clojure and ClojureScript app state since (reset)
Eclipse Public License 1.0
1.23k stars 89 forks source link

Mount stops state after changing a .cljc file #25

Closed sveri closed 8 years ago

sveri commented 8 years ago

Hi,

I was trying out 0.1.8-Snapshot as suggested and things have become better. But still I found two bugs.

The first one is that I have .cljc files in use. The code is used on clojure and clojurescript side, whenever I change the .cljc code my server component stops and does not get restarted.

I setup an example project (it is rather large, but hopefully it can help despite the noise): https://github.com/sveri/mountexample.

To reproduce:

  1. clone my example
  2. Run lein figwheel
  3. Run lein repl
  4. Run (start) in the repl
  5. Navigate to http://localhost:3000/example
  6. Open foo.bar.closp-schema
  7. Add an an extra line
  8. Switch back to browser, if you are fast enough you will see that figwheel reloads the changes
  9. Reload the browser and then you notice that the server is stopped and was not restarted.

The interesting namespaces regarding mount are: foo.bar.user and the component definitions are in: foo.bar.components.*

tolitius commented 8 years ago

@sveri thanks for a complete example, it really helps to "relive" the experience :)

looks like you have a handler that needs to be cleaned on reload, but the handler does not have a :stop method:

(ns foo.bar.components.handler... )

(defstate handler :start (get-handler config (get-tconfig))

I have not looked deeper to see how it needs to be properly stopped from the app logic point of view, but just adding an empty :stop does the trick:

(defstate handler :start (get-handler config (get-tconfig))
                  :stop :stopped)

Now if you reload the browser (per step 9.), mount would clean and redefine the handler, and the app reloads smoothly.

sveri commented 8 years ago

Hi,

Thanks, I just gave it a short try in my example and it worked. Seems like I missed it in the docs that the :stop key is mandatory. Sorry. I will give it a try later in my "real" application. It will just take some time to port it back to mount.

tolitius commented 8 years ago

@sveri,

no, you did read docs correctly: :stop is optional. however, if you reload a namespace with something that needs to be cleaned, it would need to have a :stop function

But I think we can make it better, so I just updated mount 0.1.8-SNAPSHOT to restart on namespace reload in both cases:

I tested your example, it works without adding a :stop function to the handler.

Whether or not you believe that the hander state needs to be cleaned, is up to you, and you can always add a :stop function to it in case it does.

tolitius commented 8 years ago

the updated docs.

suggestion: since you are covering both: Clojure and CojureScript, I would recommend going over this doc and switch to cljc mode, since :advanced compilation will be a lot smoother without relying on namespaces.

sveri commented 8 years ago

Hi,

I gave it another try these days and it has become better, but I still have components shut down, even without error. You can watch it happen here: https://www.livecoding.tv/video/playing-httpswwwstockfighterio-no-mic-5/ at ca. 28.00 minutes.

This is the commit I had this happen with: https://github.com/sveri/stockfighter/tree/1aaec33087944cb41829ba19aee1129734cd9586

tolitius commented 8 years ago

@sveri, thanks for the feedback. this one is a bit more involved for me to quickly follow, but here are the questions I have:

sveri commented 8 years ago

So I tried it some more extensively and see no bugs or similar things anymore. I think we can close this one too. However, there still is one difference to component, which I think is a an implementation decision.

In my setup it can happen that changes to clojurescript code will restart some parts of the backend (clojure code). I cannot track the dependencies, but most probably due to usage of .cljc files, there is a code path to the backend. This is just an assumption of course. But still, I do not see this happen with component.

sveri commented 8 years ago

Another disadvantage I see in my current application is that I have a component which stores state to the filesystem on stop and restores it on start. Automatic reloading will swipe the state (atoms) without storing and reloading them. Also this is most probably due to my architechture. Just wanted to note it here as a consideration.

tolitius commented 8 years ago

@sveri,

changes to clojurescript code will restart some parts of the backend (clojure code)

can you elaborate?

a component which stores state to the filesystem on stop and restores it on start

this is an interesting use case. Without thinking too much into it:

so here I think I would use a data store for states that needs to be stored regardless of mount OR if you still need "save on disk to resume", only states that need to be saved would have :suspend functions that would persist their states on disk, later on you could pass these states from disk to mount in args and their start functions would do something like (or existing-state (start))

sveri commented 8 years ago

I am not sure what exactly was happening there, maybe an error occured or something similar. Most probably a fault on my side.

I just did a cleanup and some refactoring and this seems to have stopped. At least for normal execution it behaves normal.

What still seems to happen is, that on compile error my atoms are reset, even although I moved them to a complete namespace now. But I will keep on researching that. Also this seems to have happened with component, but it is hard to keep track of these things and when they exactly happen.

Yes, I use filesystem instead of a DB or similar because this is only for development time and trying things out. The advantage is that I don't have to reinitialize the whole system after restarting my REPL.

From my point of view mount looks usable to me now, after fixing up all those things. I just switched back to it and will give it another try for the next few days.

This bug here is fixed and can be closed, thanks for the work :-)

tolitius commented 8 years ago

sure, thanks for reporting this. let me know if you find anything else