Fork me on GitHub
#re-frame
<
2019-01-08
>
manutter5100:01:53

I believe it's re-subscribing every time the component is rendered. If you're using a recent version of re-frame, subscriptions are cached, so it's probably not hurting you as much as you might think (but I'd still re-factor it as a Form-2, if only for "code aesthetics").

WhoNeedszZz01:01:36

I have no issue refactoring, but I only do it if there is a concrete difference. If it is indeed re-subscribing every time and that has an impact then I'll happily change it to a form-2. I just want to understand what is going on to understand why it is this way in the template.

WhoNeedszZz01:01:25

Since I'm still learning re-frame I want to establish best practices now

mikethompson02:01:17

@whoneedszzz Yes, that code should be fine. Using let to name the value coming from the subscribe is not a terrible idea. But, I also note that you only use active-panel in only one place, so you could easily replace its use with @(re-frame/subscribe [::subs/active-panel]) and remove the let. Maybe.

mikethompson02:01:01

In terms of good practice, here's some of ours: 1. Use Form-1 by default, unless for some reason you can't. 2. When using Form-1, dereference the subscription ASAP to get the value (in the let itself!). Because it is easy to later forget the @ which leads to annoyingly hard to find problems. 3. Consider using LambdaIsland Naming. https://github.com/Day8/re-frame/blob/master/docs/SubscriptionsCleanup.md#lambdaisland-naming--lin 4. If you need to use Form-2, then you can't dereference immediately, in which case we always make a naming distinction between ratoms and values by putting an * on the front of the name used for somethig which must later be derefed.

WhoNeedszZz02:01:50

Thanks for the info

WhoNeedszZz02:01:32

Personally, I don't mind the let and prefer it to be consistent in syntax for one vs multiple instances. I find it easier to read and reason about.

WhoNeedszZz02:01:24

I made it a point to remember to dereference atoms whenever you use them

mikethompson02:01:37

Yep, very understandable. In which case:

(defn main-panel []
  (let [active-panel @(re-frame/subscribe [::subs/active-panel])]      ;; added `@`
    [:> MuiThemeProvider
     {:theme custom-theme}
     [:<>
      [:> CssBaseline]
      [:> Grid
       {:container true
        :align-items "center"
        :direction "column"
        :justify "center"
        :spacing 24}
       [appbar-content]
       [show-panel active-panel]      ;; removed `@`
       [footer-content]]]]))

WhoNeedszZz02:01:41

I'd rather keep it the original way to reinforce that it's an atom being used there

mikethompson02:01:50

or

(defn main-panel []
  (let [active-panel (<sub [::subs/active-panel])]      ;;  using LIN
    [:> MuiThemeProvider
     {:theme custom-theme}
     [:<>
      [:> CssBaseline]
      [:> Grid
       {:container true
        :align-items "center"
        :direction "column"
        :justify "center"
        :spacing 24}
       [appbar-content]
       [show-panel active-panel]      ;; still no @ 
       [footer-content]]]]))

mikethompson02:01:26

If you do it your way, I believe that sooner or later you'll forget the @ and you'll waste time as a result.

WhoNeedszZz02:01:53

I appreciate the concern, but I can promise that I won't 🙂

WhoNeedszZz02:01:04

I'm big on discipline

mikethompson02:01:34

That will make you the first person in the history of Reagent. :-) But if you insist then ... I'd recommend a naming convention ..

(defn main-panel []
  (let [*active-panel (re-frame/subscribe [::subs/active-panel])]      ;; added `*` to name to make it clear it needs to be derefed 
    [:> MuiThemeProvider
     {:theme custom-theme}
     [:<>
      [:> CssBaseline]
      [:> Grid
       {:container true
        :align-items "center"
        :direction "column"
        :justify "center"
        :spacing 24}
       [appbar-content]
       [show-panel  @*active-panel]      
       [footer-content]]]]))

mikethompson02:01:14

Anyway, that's our best practice.

WhoNeedszZz02:01:57

Haha! Goals are good to have. 🙂 That's a good suggestion, thanks

aria4221:01:18

re-frame n00b question, trying to write something like this snippet which injects a value into a component based on some signal. If the result hiccup is returned in a larger body, should it re-render when the signal updates?

mikethompson21:01:32

Yes. But note that the entire "larger body" will rerender too

mikethompson21:01:00

Because @r was derefed during its render too

aria4221:01:59

Thanks, that's fine for this use-case. I must be messing something up on the subscription side, since I'm not getting a re-render I expect.

mikethompson21:01:23

perhaps re-frame-10x may supply some insight.