Fork me on GitHub
#fulcro
<
2021-02-08
>
tony.kay04:02:47

Grokking Fulcro part 2 is now available: https://youtu.be/102nHLzIbE4

đź‘Ź 30
fulcro 24
đź’Ż 21
18
Jakub HolĂ˝ (HolyJak)09:02:27

Thank you all who contributed to the Fulcro Rationale! I will now add it to the minimalist tutorial and also share it with the community.

henrik09:02:42

On the topic inputs, I'd like to revisit a question I asked previously, but didn't quite get straight in my head. Let's say I have a number of inputs, and a save button (or something else that needs to know some sum total of the inputs). Let's say I want the save button to disable and enable interactively as the user types valid or invalid values into the inputs. I only want the save button and the currently selected input to refresh. I don't want to re-render all components in the parent when the currently selected input is manipulated. It seems to me that in order to get this to work, I'd have to put the save button in a component with some ident that can be targeted independently, and give it props to know whether the inputs are valid or not. After that, I seem to have two options: 1. Use transact! and give :only-refresh the idents of the input and the save button component. 2. Use transact!! and immediately schedule a refresh of the save button component afterwards, by putting a comp/refresh-component! as the second arg to a do block. Which of these is preferable? Is there some other option that I haven't considered (barring "don't refresh the state of the save button interactively" of course, which would invalidate the entire question)

tony.kay16:02:23

1. Why do you care which inputs “refresh”. You’re in control of their state, so the question of React checking them should be irrelevant. You control the state. Do what is right in state, and trust the UI to be correct. 2. Targeted refresh requires each input have an ident, which means making quite the mess of your UI tree to get what you’re talking about. Typically a form will have an ident, and that is what you can control refreshing. If you rewrite so that inputs have idents, it means you spread your form out in a really wacky way, and have to invent idents on the fly (and not use form state). The unit of refresh control is “something with an ident” (unless you use component local state). The !! notation will get you localized refresh on closest parent IDENT, not some logical component.

tony.kay16:02:11

If you’re seeing what you think are performance problems you should first run it in prod mode to turn off inspect, guardrails, logging, and react dev.

henrik11:02:51

Yep, it's mainly a performance optimization thing. There's a big gain in only refreshing the input itself (both with :only-refresh and :synchronous? true). Unfortunately, there are some parts of the parent that does need refreshing, but far from all of it. I was hoping that I could be more surgical when issuing refreshes.

henrik11:02:09

This holds true for prod.

henrik11:02:02

> Why do you care which inputs “refresh”. You’re in control of their state, so the question of React checking them should be irrelevant. You control the state. Do what is right in state, and trust the UI to be correct. I care because it involves recalculating some pretty intense computed props when I refresh the parent wholesale. I.e., bits of the parent need to update on input, bits only need to update between interactions with input. The prior are cheap, and the latter are expensive.

henrik11:02:37

> Targeted refresh requires each input have an ident, which means making quite the mess of your UI tree to get what you’re talking about. Typically a form will have an ident, and that is what you can control refreshing.  If you rewrite so that inputs have idents, it means you spread your form out in a really wacky way, and have to invent idents on the fly (and not use form state). As it happens, our inputs (or rather, the area "just around" each input) do have natural idents, so we can target them very finely. This is the ident that transact!! ends up targeting. It never gets to the form itself, since there's a natural ident on the way there. The form also has a natural ident.

henrik11:02:06

Form state has turned out to work fine in this scenario! Unfortunately, we might need to opt out of it perhaps, which would be a bummer.

tony.kay22:02:27

So, in answer to your original question: the !! variants are about trying to get a synchronous data result on the current thread (without async submission, which is the default for Fulcro). This is an extreme optimization that you usually don’t need. See https://www.youtube.com/watch?v=102nHLzIbE4&amp;list=PLVi9lDx-4C_TBRiHfjnjXaK2J3BIUDPnf&amp;index=2&amp;ab_channel=TonyKay for the overall motivation (not needing wrapped inputs for React inputs). So, in terms of your actual base need of only refreshing a couple of things, the !! variant happens to also only bother refreshing the current component, which it assumes contains the input you’re trying to “fix”. The :only-refresh option is designed for your particular use-case, but I honestly don’t remember if the !! variant will even honor it.

đź‘Ť 3
tony.kay22:02:24

As a secondary comment on performance: Have you considered reifying (into app state) or memoizing the “expensive” parent computations?

henrik15:02:54

No, synchronous transactions don’t seem to bother with the refresh options at all.

henrik16:02:27

I assumed that recalculating props were slowing things down, but it might actually have been more subtle. We have a custom input wrapped in wrap-form-element, and I suspect it may have been misbehaving to the extent of unmounting and remounting on every refresh. I.e., every keystroke would have caused 12 or so of these inputs to go through their entire lifecycle.

henrik16:02:55

Just to get rid of that behaviour, I think that synchronous transactions, plus manual refreshes of the parent at key points (is comp/refresh-component! the correct function for this?) might just lead to fewer surprises in the long run.

henrik16:02:17

I.e., just,

(do (comp/transact!! …)
    (comp/refresh-component! parent-this))

henrik16:02:42

I just hope that the browser gods decide to be synchronous wrt the do, otherwise I'm kind of out of options.

Thomas Moerman13:02:40

I isolated a quirky situation with a union query and placeholder logic :>/bla where the final merged result is not what I would expect. The :cmmn.case/state prop is missing, as demonstrated in the test at the bottom. I'm not sure whether this is a bug or expected behaviour?

(defn get-case-ident
  [props]
  (let [case-id? string?]
    (cond
      (-> props :nexus.case.image-annotation/id merge/nilify-not-found case-id?) [:nexus.case.image-annotation/id (:nexus.case.image-annotation/id props)]
      (-> props :nexus.case.test-case/id merge/nilify-not-found case-id?) [:nexus.case.test-case/id (:nexus.case.test-case/id props)]
      :else (log/error "Cannot derive a valid ident. Invalid props." props))))

(defsc AddState [_ _]
  {:query [:nexus.case.image-annotation/id
           :cmmn.case/state]
   :ident :nexus.case.image-annotation/id})

(defsc ImageAnnotationCase [this props]
  {:query [:nexus.case.image-annotation/id
           :cmmn.case/name
           ;; cmmn.case/state -> not queried here but under :>/bla placeholder!
           {:>/bla (comp/get-query AddState)}]
   :ident :nexus.case.image-annotation/id})

(defsc TestCase [this props]
  {:query [:nexus.case.test-case/id
           :cmmn.case/name
           :cmmn.case/state]
   :ident :nexus.case.test-case/id})

(defsc CaseUnion
  "Union query component for different Case types."
  [this props computed]
  {:query (fn [] {:nexus.case.test-case/id        (comp/get-query TestCase)
                  :nexus.case.image-annotation/id (comp/get-query ImageAnnotationCase)})
   :ident (fn [] (get-case-ident props))})


(deftest bla
  (behavior "Reproduce situation where the :cmmn.case/state of the first case doesn't show up in merged props"
    (let [response [{:nexus.case.image-annotation/id "case-id-00000001"
                     :cmmn.case/name                 "Annotate Image 1"
                     :cmmn.case/state                ::merge/not-found ;; Note: I captured this by logging data-tree in pre-merge
                     :>/bla                          {:nexus.case.image-annotation/id "case-id-00000001"
                                                      :cmmn.case/state                "completed"}}
                    {:nexus.case.test-case/id "case-id-00000004"
                     :cmmn.case/name          "Test Case 1",
                     :cmmn.case/state         "active"}]]

      (assertions
        "Show that :cmmn.case/state isn't merged"
        (merge/merge-component {} CaseUnion response) =>
        {:nexus.case.image-annotation/id {"case-id-00000001" {:nexus.case.image-annotation/id "case-id-00000001"
                                                              :cmmn.case/name                 "Annotate Image 1"
                                                              ;; :cmmn.case/state             "completed" <== EXPECTED!!
                                                              :>/bla                          [:nexus.case.image-annotation/id "case-id-00000001"]}}
         :nexus.case.test-case/id        {"case-id-00000004" {:nexus.case.test-case/id "case-id-00000004"
                                                              :cmmn.case/name          "Test Case 1"
                                                              :cmmn.case/state         "active"}}}

        ))))
I'm guessing that the ::merge/not-found indicator that results from the union query, interferes with merging the data under the placeholder, that happens to have the same ident as the parent component. PS:@tony.kay would it be useful to log this as a github issue? Even if it is not a Fulcro bug, it could be useful to track for future reference documentation.

tony.kay18:02:20

I do not have time to look into this right now. Since you seem to have a repro case and test you could just open an issue, yes.

tony.kay18:02:54

in terms of it being expected or not…I’m not saying yes/no…I just don’t have time to read all that period

Thomas Moerman19:02:28

Ok, I'll clean it up some more and log an issue. I don't think it's a pressing issue, more of an edge case to be aware of.

đź‘Ť 3
Carlo14:02:15

since there's not a guardrails channel, I'll ask here: is there a way of creating a normal value with a spec (like def)? It seems that >def is used to create specs, non specced values

tony.kay19:02:56

Values do not have specs. Call sites and map values have specs (the latter via their keys)

tony.kay19:02:10

values are checked by specs

Carlo20:02:51

sorry, I probably used some slightly wrong terminology: by value I didn't mean an atomic one, but also a map value for example. Basically, I can define a value and then call conform to see if it obeys some spec. I was interested in understanding if I could do that when I define my map, for example.

Carlo14:02:46

also, when I trigger a spec error, is there a way of getting back a stack trace of the functions that triggered it?

tony.kay19:02:46

You can use timbre log/error and hand it an exception (log/error (ex-info …) "Message")

tony.kay19:02:58

timbre will show the stack trace then (first arg is exception)

Carlo20:02:17

awesome, I'll look into that, thanks!

Braden Shepherdson18:02:10

Has anyone tried to connect Fulcro with Firestore (from the client side, no server)? That's the setup I'm working with and considering using Fulcro. It feels like Firestore could be treated as a remote, and any real time updates simply fed to the app as load!s. Is there any prior art here?

tony.kay18:02:23

Not a ton, but the rad-kv-store is a decent first step to getting there for RAD-based stuff

Braden Shepherdson18:02:31

I'm a bit worried about spamming Firestore with reads that aren't necessary. But perhaps if I write it properly the queries never hit Firestore, and instead I just request all the working data up front, and let Firestore subscriptions keep the client DB up to date. That's not going to be practical in every app, but it is for my two use cases.