Fork me on GitHub
#fulcro
<
2020-01-13
>
thosmos01:01:50

I’m learning how to use F3 UISMs. How do you access an actor’s full denormalized tree of data? I added an aliased key :session/data to the session component that has a number of joins. The data is getting normalized into the DB fine, but when I use (uism/alias-value env :session/data) I just get one level of data with ident edges …

thosmos01:01:59

This worked:

(fdn/db->tree
  (comp/get-query (uism/actor-class env :actor/current-session))
  (uism/actor->ident env :actor/current-session)
  (::uism/state-map env))
Does this seem reasonable? I realize this makes my session handler dependent on the implementation details of this extra user data. I’m not sure how to bring in a fairly large tree of data and do some processing without this. Maybe this is where having another state machine that is more custom for this specific user data is useful, and that would get started once the user data is loaded? I also realize though this idea of separation of concerns between UISM and UI is intended for reuse of the state machine, but not necessary if that’s not a goal, true?

👍 4
tony.kay05:01:58

Pretty much all of that. There is nothing keeping you from making it a mess of cross dependency. The tools at your disposal are just that: tools.

👍 4
Chris O’Donnell04:01:29

I'm running into an odd bug with semantic-ui-wrapper; wondering if anyone has any ideas. For whatever reason, when I pass in an error prop, it doesn't update even though the component seems to be rerendering. Here's the component:

(defsc GiftForm [this {::gift/keys [name] :as gift}]
  {:query [::gift/id ::gift/name fs/form-config-join]
   :ident ::gift/id
   :form-fields #{::gift/name}}
  (js/console.log gift)
  (js/console.log "invalid" (fs/invalid-spec? gift ::gift/name))
  (dom/div {}
    (ui-form {:onSubmit (fn [evt]
                          (comp/transact! this [(model.gift/create-gift gift)])
                          (m/set-string! this :ui/gift-name :value ""))}
      (ui-form-field {}
        (ui-form-input (cond-> {:placeholder "A pony"
                                :onChange #(m/set-string! this ::gift/name :event %)
                                :onBlur #(comp/transact! this [(fs/mark-complete! {:field ::gift/name})])
                                :fluid true
                                :value name}
                         (= :invalid (fs/get-spec-validity gift ::gift/name))
                         (assoc :error name))))
      (ui-button {:type "submit"
                  :primary true}
        "Submit"))))
The component updates its value properly when I type in it. As soon as I empty the input (the spec is valid for a nonblank string), an empty error message renders. The error message doesn't update when I type in the input, but stays there and empty even after the value becomes valid again. The console/log prints what I would expect wrt. the validity of the input value. I'm at a loss for why the error prop doesn't seem to update; open to any ideas.

tony.kay05:01:25

If I remember right you have to set error on the form, too @codonnell

tony.kay05:01:07

I think the idea with SUI is that fields might just declare their errors all the time, and something else could decide at the form level to show/hide them

Chris O’Donnell11:01:36

The form error prop seems to control whether error messages are displayed, but does not seem to have an effect on the error state of inputs. (Thanks for the tip; I had not realized that.) I found a workaround, but still don't understand the strange behavior of the not-updating error prop. If, instead of doing (cond-> props error? (assoc :error true)) I do (assoc props :error error?), the error prop updates properly. Perhaps this has something to do with how the react component is wrapped?

tony.kay15:01:38

Well, the only difference is that your error binding might not be a boolean.

tony.kay15:01:09

cond-> is a macro that rewrites the first to (nearly) the second…other than you’re using the value of error? instead of a true

Chris O’Donnell18:01:36

Sorry, I was paraphrasing a bit. The difference between the two maps in practice is the working map has :error false while the broken one had no :error key.

Chris O’Donnell18:01:01

In the case that the name value was valid.

Chris O’Donnell18:01:25

Both maps were the same in the case that there was an error.

Chris O’Donnell18:01:16

I will see if I can put together a minimal repro this evening.

thosmos21:01:37

what is the value of error? If it’s ever anything other than false or nil then it will give a false positive to the test

thosmos21:01:17

oh because the broken one has no :error key that suggests that the component that’s looking for that might be where the problem is.

Chris O’Donnell00:01:54

@tony.kay Put up a minimal repro of the issue at https://github.com/codonnell/form-error-repro. Would love it if you could take a look when you get a chance. I don't have time right now, but I'm also happy to dig in and see if I can narrow down the underlying cause.

thosmos21:01:23

I think it might have something to do with the Fulcro’s dom/wrap-form-element and how it uses React’s local state to track values. The problem with the non-working one is it only sets a value when there’s an error, but because you remove the :error field from the map, the wrapper doesn’t know to set the value to false, because it’s just not there. It’s similar to how datomic only removes a field when you explicitly tell it to, where as if you just transact another map with that missing field, it assumes you want to only merge the fields that are present. That’s my read on the problem … Your working version looks more idiomatic … basically declare all the dom attributes that you’re going to be changing and then update the values but not the presence of the attributes.

thosmos23:01:39

Actually the problem is generally as I described above, but it appears to be React itself that isn’t removing the key when fulcro passes in a new state object that is missing that key. So again, it’s best to just have a fully controlled component …

Chris O’Donnell23:01:54

You're one step ahead of me in diagnosing this. Was just able to confirm your assertion that the key is not removed from component local state:

Chris O’Donnell00:01:42

Okay, I understand now. The problem is here: https://github.com/fulcrologic/fulcro/blob/develop/src/main/com/fulcrologic/fulcro/dom.cljs#L130. .setState does a merge and does not remove keys that are present in the previous state and missing from the subsequent one.

Chris O’Donnell00:01:37

IIRC these wrappers use local state so that the input updates immediately instead of waiting for the queued render.

thosmos02:01:04

I made a fix to the fulcro code to handle this, and I upgraded it to get rid of the deprecated react lifecycle. @tony.kay what do you think of this? should I make a PR or an issue first? https://github.com/thosmos/fulcro/commit/ebe4d6e83f2c8b9e9bfc3256dd833851b51040f8

Chris O’Donnell02:01:50

An alternate approach that gets around the shallow merge issue by storing state as an object like {"data" state} instead of just state: https://github.com/codonnell/fulcro/commit/2542259d5fce6d6ca8221e6f869c8e5d9580d657

thosmos02:01:41

i like your solution better than mine, but mine does work with componentDidUpdate which needs to happen eventually

thosmos02:01:07

but I think your approach would be easier to diff

tony.kay04:01:09

@U07KXN95L your PR is backwards the UNSAFE one is the one that will be around…componentDidUpdate is going away

tony.kay04:01:24

I had already upgraded for it to be correct with respect to that

tony.kay04:01:18

and I won’t merge a PR on this until someone has used it in a real app for a decent amount of time/testing…this bit of code is easy to break, which pretty much screws every production app on the planet that uses Fulcro…

tony.kay04:01:01

and it breaks, unfortunatly, in ways that are very hard to reason about because of async js, the way React works, etc.

tony.kay04:01:22

sorry @U07KXN95L, I see what you’re trying to do…componentDidUpdate isn’t going away, but changing from the one I’m using to that one will have strange consequences I fear

thosmos04:01:43

yeah I get it about needing to be sure it works before merging it. I’m not sure how to ensure that. We’re dealing with 2 things in my commit, the failure to clear state for props that disappeared, and the transition to the stable lifecycle function. I think @codonnell’s fix for the missing props is probably better, which solves it just by moving the component’s state into a :data child key, assuming that other things don’t depend on those prop fields being in the root of state. Is that possible? if so, my fix is better because it keeps the fields in the root of state. As for componentDidUpdate, the challenge is it gets called both when props and state change, so we need to diff props to make sure it’s necessary to call setState, otherwise an endless loop happens. Do you think a lot of folks are depending on dom/wrap-form-element? It seems like a relatively predictable set of changes …

thosmos04:01:19

but not changing to componentDidUpdate will have even worse consequences when react 17 rolls out …

thosmos05:01:58

oh I see, the UNSAFE versions will be around, just not the originals

thosmos05:01:18

in that case, your fix for missing props is probably the better way to go

thosmos05:01:43

getDerivedStateFromProps might be a better way to go for the props diffing?

Chris O’Donnell05:01:09

Perhaps. But if this is a low priority change for Tony, I think the smallest, least risky diff makes the most sense.

thosmos07:01:25

Here’s a more minimal fix that does the bare minimum without changing how fields are currently stored in state https://github.com/fulcrologic/fulcro/pull/359/commits/29da3042e0159a60d1717d30175ac58e2acc5fd9

tony.kay15:01:14

don’t love that from a performance perspective…two new reduces on every key of props on every input?

tony.kay15:01:51

So, there are many reasons you can’t overwrite state now, and apps in general need to be written to tolerate this. Since what we’re doing is passing values to a react component as props, what we should be doing is storing the cached data in a sub-key that we can replace. Then there is no need for this reduction mess

tony.kay15:01:58

oh, @codonnell @U07KXN95L I think I already have written a fix to this, but just had not converted the old DOM stuff to use it to avoid breaking changes….the new StringBufferedInput stuff…let me look to see if it has the same problem.

tony.kay15:01:43

actually that code has similar problems I think, but I’d feel better about fixing and testing it.

tony.kay15:01:59

It’s in …dom.inputs ns.

tony.kay15:01:26

it is meant to allow for auto type conversions on input values, but that could just be identity

tony.kay15:01:53

we could also feature flag a fix with goog-define and keep the old impl as the default for now, and use compiler setting to choose new implementation until we’ve used it enough to feel safe

tony.kay15:01:05

I’ll do that real quick

tony.kay15:01:42

hm…so, it looks like perhaps just using a subkey on state might be the easier fix…

tony.kay15:01:50

just nest the props in a sub-key, so we’re in control of the merge semantics of the value. No logic changes at all, just change where the value is stored very slightly

tony.kay15:01:19

Sorry, @codonnell I missed your patch

tony.kay15:01:23

you were doing exactly that

tony.kay15:01:28

Yes, that is the correct fix

tony.kay15:01:15

@codonnell I’m not sure why you are doing the object assign stuff…Am I missing something with just this level of patch? https://github.com/fulcrologic/fulcro/pull/360/files

tony.kay15:01:53

With yours I have to reason about it a lot more to try to figure out if it is the same…I want to just change the path so I’m in control of the value (instead of being subject to the shallow merge).

Chris O’Donnell16:01:51

gobj/extend did not behave as I expected. Can't remember exactly how at the moment; I could check after work today. Since it's deprecated in favor of Object.assign, I replaced it with that, and it worked according to my expectations.

Chris O’Donnell16:01:38

Something like the object under data was nil when I expected it to have a value @tony.kay

tony.kay16:01:06

Hm. OK, well, the version I did is on develop now, and I’m working with it. Did not realize it is deprecated, and would be interested to know what the specific issue was.

thosmos20:01:08

Sounds good. Glad it got sorted

Chris O’Donnell02:01:07

@tony.kay The issue with gobj/extend was PEBKAC. I didn't realize it returns void instead of the object that was extended, unlike js/Object.assign. I tried to use its return value. facepalm I don't think there is any reason to use my patch over yours; you accomplished the same thing, but more cleanly. Thanks for taking the time! I'm just working on a little side project that's unlikely uncover weird edge cases, but I did bump my dep to the snapshot and will let you know if I see anything odd.

tony.kay02:01:33

thx…I doubt there is anything wrong, since it was such a non-invasive change

👍 4
otwieracz13:01:55

Sorry for newbie question, but I have hard time figuring how should I use things like react-bootstrap with Fulcro.

otwieracz13:01:28

Should I even be using it?

otwieracz13:01:44

As dom is provided by fulcro, I am not convinced that I should be able to use Bootstrap.. But from the other hand, it's hard to believe that I won't be allowed to use stuff from npm.

Björn Ebbinghaus13:01:16

@slawek098 Sure, why not? When using shadow-cljs you can simply require from npm sources:

(:require 
  ["react-bootstrap/Button" :refer [Button]])

; somewhere I want to use the component:
(Button #js {:variant "primary"})

; The above code won't work
; see: 

otwieracz13:01:41

OK, I will read it - thanks!

otwieracz14:01:24

@mroerni Hmm, link you have shared suggests using ui factories, etc.

Björn Ebbinghaus15:01:32

Ah, yes... react-icons (the only npm lib I am using) provides the components as functions... In this case this works:

(:require 
   ["react-bootstrap" :refer [Button]])

(def ui-button (interop/react-factory Button))

(ui-button {:variant "danger"} "Hello World")

otwieracz15:01:03

Yes, I've just managed to come to the same conclusion 🙂

otwieracz15:01:14

There's no way to avoid factory creation?

otwieracz15:01:47

I remember using [:> Button] syntax some time ago with Hiccup IIRC.

Björn Ebbinghaus15:01:22

You can write

(:require 
  ["react-bootstrap" :refer [Button]]
  ["react" :as React])

(React/createElement Button #js {:variant "danger"} "Hello World")
But this is just a dumber version of what the interop ns does.

otwieracz15:01:03

OK, so nevermind.

otwieracz15:01:27

Thanks for help!

thosmos21:01:49

In the past I’ve done something like this, but I’m not sure if it still works:

(ns my.ui.page
  (:require 
    ["react-bootstrap" :refer [Button]]))

(defn r
  "Wraps a React component into a Fulcro factory and then calls it with the same args"
  [react-class & args]
  (apply (interop/react-factory react-class) args))

(div 
  (r/Button {:variant "danger"}))

thosmos21:01:31

@slawek098 are you aware that Fulcro3 has good bindings for Semantic-UI React?

[com.fulcrologic.semantic-ui.modules.dimmer.ui-dimmer :refer [ui-dimmer]]
    [com.fulcrologic.semantic-ui.elements.input.ui-input :refer [ui-input]]
    [com.fulcrologic.semantic-ui.collections.form.ui-form :refer [ui-form]]

otwieracz06:01:37

No, I weren't aware.

otwieracz06:01:22

And that's great, as I even considered it!

otwieracz14:01:42

There's nothing about directly using Button as function.

tony.kay15:01:27

because Button is a class. JSX is transpiled to a call to a createElement on that class. Button isn’t a function in JSX either.

tony.kay15:01:54

read React docs…Fulcro’s doing nothing special here…it’s React

tony.kay15:01:08

you’re essentially complaining that you don’t have a DSL to transpile using webpack et al…trust me, you’re better off 😜

otwieracz15:01:54

How should I understand error message: cljs$core$ExceptionInfo {message: "Invalid join, {:wire-lists nil}", data: {…}, cause: null, name: "Error", description: undefined, …}?

Björn Ebbinghaus15:01:21

Like it states. There is an invalid join Instead of nil there should be a vector. What is your query for the component that causes the error?

otwieracz16:01:24

I am just looking deeper into it, because I don't understand something..

otwieracz16:01:40

(defsc Wire [_this {:wire/keys [label color connections]}]
  "Single wire component, representing point-to-point electrical connection."
  {:query         [:wire/id :wire/label :wire/color :wire/connections]
   :ident         :wire/id
   ;:initial-state {:wire/id          :param/id
   ;                :wire/label       :param/label
   ;                :wire/color       :param/color
   ;                :wire/connections :param/connections}
   :initial-state (fn [{:keys [id label color connections]}] {:wire/id          id
                                                              :wire/label       label
                                                              :wire/color       color
                                                              :wire/connections connections})}

otwieracz16:01:51

(comp/get-initial-state Wire {:id :foo :label "Foo" :color "black" :connections [1 2 3]})
=> nil

otwieracz16:01:20

I am probably missing something, as it's hard for me to wrap my head around fulcro

otwieracz16:01:53

Oh! Obviously you can't have docstrings there.. 🙂

Björn Ebbinghaus16:01:50

👍 I recommend using clj-kondo for this and setting

:misplaced-docstring {:level :error}
🙂 Done this mistake way to often.

otwieracz17:01:41

What's the difference between those two forms of initial state specification:

(defsc WireList [_this {:wire-list/keys [wires]}]
  {:query         [:wire-list/id {:wire-list/wires (comp/get-query Wire)}]
   :ident         :wire-list/id
   ;:initial-state {:wire-list/id    :param/id
   ;                :wire-list/wires [{:wire/id          0
   ;                                   :wire/label       "Foo"
   ;                                   :wire/color       "#000"
   ;                                   :wire/connections [0 1]}]}
   :initial-state (fn [{:keys [id]}] {:wire-list/id    id
                                      :wire-list/wires [(comp/get-initial-state Wire {:id          0
                                                         :label       "Foo"
                                                         :color       "#000"
                                                         :connections [0 1]})]})
Produced initial state differs like:
(comp/get-initial-state WireList {:id :foo})
=> {:wire-list/id :foo, :wire-list/wires [{}]}
(comp/get-initial-state WireList {:id :foo})
=>
{:wire-list/id :param/id,
 :wire-list/wires [{:wire/id 0, :wire/label "Foo", :wire/color "#000", :wire/connections [0 1]}]}

Björn Ebbinghaus09:01:44

From http://book.fulcrologic.com/#_options_lambda_vs_template > The core options (`:query`, `:ident`, `:initial-state`) of `defsc` are special. They support both a lambda and a template form. The template form is shorter and enables some sanity checks; however, it is not expressive enough to cover all possible cases. The lambda form is slightly more verbose, but enables full flexibility at the expense of the sanity checks.

otwieracz10:01:19

I have read this obviously - but I thought that this initial state form I am looking for is just standard case supported by template form.

pithyless21:01:45

@slawek098 It may not be immediately obvious, but functions like get-query and get-initial-state are necessary when doing joins, because they add metadata that other Fulcro operations are looking at. So, in general, these are not equivelant:

{:wire-list/wires [{:wire/id 0}]}

{:wire-list/wires [(get-initial-state Wire {:wire/id 0})]}
(Just something to keep in mind). In your specific case, if this is working for you:
:initial-state (fn [{:keys [id]}]
{:wire-list/id    id
 :wire-list/wires [(comp/get-initial-state Wire {:id 0, :label       "Foo"})]})
Then the equivelant template would be along the lines of:
:initial-state 
{:wire-list/id    :param/id
 :wire-list/wires [{:param/id 0, :param/label       "Foo"})]})
Although, I don't think this will work (because the :param/* substitution happens first, so you're passing in 2 unrelated ids as :id. In fact, you probably need:
:initial-state 
{:wire-list/id    :param/id
 :wire-list/wires [{:wire/id 0, :wire/label       "Foo"})]})
^ This implies you may need to change how you're currently defining :initial-state for Wire.

pithyless21:01:26

Not really sure if those two nested :param/id 's would work - it would be good if someone could confirm that; or I'll just test it tomorrow out of curiosity.

tony.kay19:01:45

Fulcro 3.1.4 released, along with new binaries of the Electron version of Fulcro Inspect 2.2.1. This new version of electron-based Inspect requires 3.1.4 to work properly. It fixes a few known issues: You can now connect any number of apps to the electron app at once, disconnects are properly processed and cleaned up, and you can now set the port on which the electron version listens.

🎉 12
tony.kay19:01:09

I am only able (willing?) to test on MacOS, so Linux and Windows binaries (which are generated by electron-build for me) are there for the community, but have not been tried.

cjmurphy20:01:12

I'm on Linux. I needed to uninstall the working alpha version - so did a fresh install of this one. This one doesn't start up. (as in Right click, 'New Window', will never bring up a new window). Trying to think where the log files might be...

cjmurphy22:01:08

ps aux shows that there are three processes, each at /opt/fulcro-inspect-electron/fulcro-inspect-electron . But no UI.

cjmurphy04:01:47

When comparing with the alpha version, it has an additional two processes (so five altogether). These two both have --type=renderer on their arguments. So somehow two renderer processes are not instantiating on the latest version.

tony.kay06:01:07

@U0D5RN0S1 perhaps you could check out the source…the DEVELOPMENT.md talks about running it. I wonder if it is the dist or something else.

tony.kay06:01:50

and you upgraded Fulcro to 3.1.4, right?

cjmurphy06:01:59

Yes indeed. But that can't be the issue. My problem is getting it up, seeing its UI, which has nothing to do with Fulcro.

tony.kay06:01:41

So, anyway, it is pretty easy to run from source

tony.kay06:01:47

and you’d see logs there easily if it is a general problem instead of a binary dist problem

tony.kay06:01:58

you using .deb or AppImage?

tony.kay06:01:46

Try the AppImage

cjmurphy06:01:56

Didn't know I could try AppImage. Will do...

tony.kay06:01:16

Yeah, that’s a self-contained linux executable

tony.kay06:01:23

supposed to work on most distros

tony.kay06:01:41

just set execute permission on the file and go

👍 4
cjmurphy06:01:58

I don't get anything from window manager starting. Tried from the console and got a error message.

cjmurphy06:01:14

A JavaScript error occurred in the main process Uncaught Exception: Error: Cannot find module 'electron-settings' at Module._resolveFilename (module.js:543:15) at Function.Module.resolveFilename (/tmp/.mountfulcrofNcLUt/resources/electron.asar/common/reset-search-paths.js:35:12) at Function.Module._load (module.js:473:25) at Module.require (module.js:586:17) at require (internal/module.js:11:18) at /tmp/.mount_fulcrofNcLUt/resources/app.asar/js/background/main.js:2202:333 at Object.<anonymous> (/tmp/.mount_fulcrofNcLUt/resources/app.asar/js/background/main.js:3632:3) at Object.<anonymous> (/tmp/.mount_fulcrofNcLUt/resources/app.asar/js/background/main.js:3634:3) at Module._compile (module.js:642:30) at Object.Module._extensions..js (module.js:653:10)

tony.kay06:01:45

oooohhhh…I bet my docker linux image for build needed update package json deps 😜

tony.kay06:01:51

that is helpful…just a min

tony.kay06:01:42

@U0D5RN0S1 I’m uploading a new AppImage now. I’m headed to bed, so I’ll check back to see if that one worked for you tomorrow.

tony.kay06:01:25

@U0BR5D7A6 looks like you’re not the only one seeing a problem.

tony.kay06:01:43

can you try creating that missing file as an empty on @U0BR5D7A6?

tony.kay06:01:25

like with touch ~/Library/App*Support/fulcro-insp*/Settings

tony.kay06:01:46

@U0D5RN0S1 new AppImage is up

cjmurphy06:01:09

Got it, will report back, Thanks.

tony.kay06:01:10

basically the new electron-settings addition is trying to save/load the websocket port from a settings file. I do not pre-create it

tony.kay06:01:49

seems like that is failing in OSX, but I must have accidentally created it…so I didn’t see the problem that @U0BR5D7A6 is seeing…and you’re seeing a missing node module file.

tony.kay06:01:06

there are times when I miss static compiles…

cjmurphy06:01:05

2.2.1 d/loading now. I'll try it out a bit later.

tony.kay07:01:00

@U0D5RN0S1 ignore that AppImage

tony.kay07:01:04

I did not compile it right

tony.kay07:01:12

@U0BR5D7A6 good to know, thx

tony.kay07:01:08

actually, maybe I did…anyway, I think I have a fix for the macos one already done

tony.kay07:01:21

but I’ll do more releases tomorrow…thanks for the testing!

tony.kay07:01:15

@U0BR5D7A6 Whatever…new release up for macos 🙂

tony.kay07:01:22

can’t leave it undone 😜

tony.kay07:01:35

also uploading new linux and windows binaries

levitanong07:01:02

@tony.kay lol you know what’s weird? If i rm the file, it still works. I haven’t installed the new version yet. o_O

cjmurphy10:01:18

FI working for me again (so on Linux) 🎉

tony.kay16:01:35

great, so good on MacOS and Linux