Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

subscribers aren't triggered when subscriptions are reloaded via figwheel #60

Closed
aaronjensen opened this issue May 17, 2015 · 13 comments
Closed

Comments

@aaronjensen
Copy link

If I make a change to a sub, I have to reload the page or otherwise change the state in order for the sub to get triggered. Ideally, the subs should fire if they're reloaded.

@mike-thompson-day8
Copy link
Contributor

I might need a bit more information. Does this reloading involve figwheel? If it does, what are doing in your on-jsload fucntion?

@mike-thompson-day8
Copy link
Contributor

BTW, both re-frame examples now better demo a figwheel workflow. But that cleanup is only in the develop branch for the moment.

@aaronjensen
Copy link
Author

Indeed.

I had this, which may be old config:

(figwheel/watch-and-reload
  :websocket-url "ws://localhost:3449/figwheel-ws"
  :jsload-callback core/mount-root)

@mike-thompson-day8
Copy link
Contributor

I still don't know what core/mount-root does. If there is a problem it will be what you have done in there.

Please have a careful look at the various examples. Like this one:
https://github.com/reagent-project/reagent-template/blob/master/src/leiningen/new/reagent/src/cljs/reagent/core.cljs#L47-L48

Notice how mount-root completely re-renders from the top down.
(reagent/render [current-page] (.getElementById js/document "app")

I'm going to close this, because it is not really not a re-frame issue.

@aaronjensen
Copy link
Author

Ah, sorry, I thought that was from a re-frame template.

My mount-root is

(defn mount-root
  []
  (reagent/render [my.views/my-app] (.getElementById js/document "app")))

So yes, it rerenders the entire page, at least in theory. In practice, it doesn't even call my.views/my-app, likely because its result is cached. Note that everything works properly if the views.cljs file is the one changing, but if it's a file w/o the views, then the render is effectively a no-op.

@aaronjensen
Copy link
Author

After digging in though, you're right, it's not a reframe issue. I thought it had to do w/ the subscriptions, but it actually has to do w/ reagent/render doing its job. I'll check w/ reagent to see if there's a way to blow the cache.

@aaronjensen
Copy link
Author

For posterity, I tossed ^:figwheel-always on the views file namespace and that takes care of things.

@mike-thompson-day8
Copy link
Contributor

It is puzzling that was needed. As I understand it, you are modifying a subscription handler, right, so that subs.cljs file will be reloaded, and then mount-root is being called by figwheel.

@aaronjensen
Copy link
Author

So here's the real fix:

(defn mount-root
  []
  (reagent/render [#'my.views/my-app] (.getElementById js/document "app")))

Note the #'

See: https://github.com/reagent-project/reagent/blob/ff10d235911a60135d52c18fced9f071a7d8e366/src/reagent/core.cljs#L100-L104

@mike-thompson-day8
Copy link
Contributor

Again, I'm puzzled as to why that would be a real fix. You aren't calling force-update-all you are force rerendering the entire DOM - a sledge hammer (which is good).

For further background:
reagent-project/reagent#94
reagent-project/reagent#112

@aaronjensen
Copy link
Author

Here's a repro. You can ignore the piggieback stuff, just check the links. I'm guessing the problem has to do with memoized ratoms, but i haven't dug too deep.

https://github.com/aaronjensen/figwheel-plus-vim-fireplace

@vganshin
Copy link

I ran into the issue with shadow-cljs, and while googling the solution I found this issue. Then I found re-frame.core/clear-subscription-cache! function in the source code, which should be on every code reloading while development before React re-rendering. Maybe it will save time for someone.

P.S. See also Why clear sub cache note.

@mike-thompson-day8
Copy link
Contributor

@vganshin I also would suggest creating an app using re-frame-template and noticing the way that it structures things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants