reagent-project / reagent-template

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

Easy Mistake To Make #43

Closed mike-thompson-day8 closed 9 years ago

mike-thompson-day8 commented 9 years ago

This is an easy mistake to make: https://github.com/reagent-project/reagent/issues/112

I wonder if this template should contain a comment in the right spot, warning the programmer about this issue, including perhaps a link to the docs?

yogthos commented 9 years ago

agreed, would you be up for doing a pr for this? :)

mike-thompson-day8 commented 9 years ago

I'll see if I can make some time. BTW, this was previously reported via #36

mike-thompson-day8 commented 9 years ago

Are these notes sufficient (see the split of init!)? https://github.com/reagent-project/reagent/issues/112#issuecomment-82908212

yogthos commented 9 years ago

That seems descriptive enough, although mount-root isn't actually used now.

mike-thompson-day8 commented 9 years ago

mount-root is used in two places.

yogthos commented 9 years ago

Right, but I don't think that's currently necessary since ^:figwheel-no-load is used in the dev.cljs file so that figwheel doesn't try to reload it each time.

mike-thompson-day8 commented 9 years ago

Righto, I'll close this with no clarifications and changes made. But just so we are clear: three people have now reported this issue as very confusing, and they have burned time trying to understand it (to the point that they have raised questions across in the main reagent tracker). But if you are quite sure there is nothing further that can be done to clarify, we'll have to leave this here.

yogthos commented 9 years ago

I think the best option would be to describe the figwheel behavior in the docs to be honest. The original issue is what spurred adding the ^:figwheel-no-load flag to make thing behave as expected as well.

My understanding is that the original problem shouldn't be happening with the current version, but the confusion is around what gets reloaded and when. Perhaps I'm misunderstanding?

mike-thompson-day8 commented 9 years ago

I believe you are misunderstanding. As I understand it, this has nothing to do with ^:figwheel-no-load

This template's use of force-update-all IS A BUG for all the reasons explained in the reagent issue. @holmsand says don't call force-update-all. People are being surprised and baffled because this template calls force-update-all which won't rerender the root component (as explained in regent issue).

This is the line that's the problem:
https://github.com/reagent-project/reagent-template/blob/4f0df9c23fe803ab623848d480ca884890cfeed9/src/leiningen/new/reagent/env/dev/cljs/reagent/dev.cljs#L11

That line should be calling the proposed core/mount-root instead of force-update-all

yogthos commented 9 years ago

Ah ok, I see what you mean now. I've pushed out a new template using the mount-root approach.

mike-thompson-day8 commented 9 years ago

Thanks!