Fork me on GitHub
#fulcro
<
2017-10-26
>
tony.kay17:10:42

So, as an unofficial thing: I’m strongly considering dropping Om Next as a dependency. I’ve written a SNAPSHOT version of Fulcro 2.0 with internals that are based upon it, but simplify it and fix a number of things. I’ve been trying to find a way to patch Om Next itself, but the changes I want to make are just too invasive, and I think we can get a better result by dropping the baggage Om Next has related to pluggable dbs (we don’t need it, but it complicates things quite a bit). I’ve pushed fulcro 2.0.0-SNAPSHOT and a companion snapshot of fulcro-spec to clojars. *THIS IS NOT a production-ready version. I want to put it out there * See https://github.com/fulcrologic/fulcro/blob/2.0/README-fulcro-2.0.adoc

tony.kay17:10:19

I’ve run it against all of the existing demos, cards, tests, and the template. I know of the following two things that are broken: 1. Server-side rendering (building initial state) 2. component-local state updates may cause children to look as if they go back in time.

tony.kay17:10:02

The cards build has dynamic queries on the initial-app-state-card

tony.kay17:10:52

I’d love it if anyone wants to try it out. I’d be particularly interested in benchmarks.

tony.kay17:10:28

The query engine should have less overhead, though the UI refresh could possibly be slower. I’m hoping the trade off results in generally faster overall performance

tony.kay17:10:57

There is a 2.0 branch of fulcro-template that has already been ported as well…but with SSR disabled

tony.kay17:10:38

Oh, and 2.0.0-SNAPSHOT of fulcro-spec

tony.kay17:10:49

so, if you use that, you’re covered without having to remove it

roklenarcic17:10:47

Hopefully your new freedom will let you improve routing behaviour

tony.kay17:10:02

@roklenarcic what’s wrong with routing behavior?

tony.kay17:10:11

that should be working fine

roklenarcic17:10:12

filling up initial state

roklenarcic17:10:24

only grabs the first route

roklenarcic17:10:01

and router's ident "eats" the ident of the component that's at route top-level

roklenarcic17:10:32

I've always considered routing in om.next a bit hacky

tony.kay17:10:50

um….have you read M15 section of the devguide?

tony.kay17:10:37

as far as I know routing works perfectly without any problems. If it is broken, it is a bug in Fulcro. Assuming you’re talking about defrouter

roklenarcic17:10:39

It's just a bit.... opaque

tony.kay17:10:57

I prefer “abstract”.

tony.kay17:10:10

but I still don’t understand your complaint above.

tony.kay17:10:30

as far as I know routing gets all routes state into initial app state, and nothing gets “eaten” that I know of

tony.kay17:10:03

so, I just don’t understand your comment :thinking_face:

roklenarcic18:10:33

I guess I was doing something wrong then

tony.kay18:10:15

Were you using defrouter or hand-coding it?

roklenarcic18:10:42

I'll test some more

tony.kay18:10:31

ok. just so you know, nothing about Om Next should be breaking that one. The dynamic router is technically wonky, but also unfixable without making Om Next’s dynamic queries serializable (thus the desire to fork)

tony.kay18:10:20

With Fulcro 1.0 if you do code splitting and use dynamic router (or dyn queries at all) you lose support viewer, and hot code reload gets ugly.

roklenarcic18:10:37

I guess I need to work out how to manage normalisation better. Here an example of what I was doing today: I tried to show person edit modal. So I made a mutation that assoced into [:person-edit :singleton] {:modal-item [:person/by-id 5]}. That was missing modal initial data, but if I just added b/Modal initial state to that person in another mutation, it wouldn't work because modal state wasn't normalised to fulcro.boostrap..../modal-by-id table. So I had to unpack the person into tree form, add modal initial state and then move it back into db form, by writing this: `(defn update-comp-state [old-state comp ident update-fn] (as-> (om/db->tree [{ident (om/get-query comp)}] old-state old-state) data (get data ident) (update-fn data) (fc/merge-component old-state comp data)))` now it works

tony.kay18:10:13

Did you follow the example in the devguide? You should have your editor overlay the person

tony.kay18:10:29

line 369 of section N15's source

tony.kay18:10:49

parallel query of the modal and the data to render.

tony.kay18:10:15

You do have to point the field (via an ident) at the person in app state

tony.kay18:10:33

but normalization should be something that happens on load or initialization

roklenarcic18:10:09

I had that initially and it worked, but then I wanted to display name of person in modal header

tony.kay18:10:27

so, why did you change the structure?

tony.kay18:10:38

you can totally pull the data out and put it in the header without changing the query

roklenarcic18:10:07

you mean pull the name from joined in person in props?

tony.kay18:10:13

sure…it’s just a map

tony.kay18:10:21

this isn’t OO 😜

tony.kay18:10:28

information isn’t hidden

roklenarcic18:10:45

I felt I was breaching the old "this component shouldn't know about structure of things in subqueries"

tony.kay18:10:10

yeah, we programmers get stuck on rules…let me see if I can ’splain it better 🙂

roklenarcic18:10:23

then when subquery changes it can break the parent component

tony.kay18:10:38

sure…but hold on…

tony.kay18:10:30

from a composition perspective, you need to ask the child for the query. That is true. The parent, however, always has to know stuff about its immediate children: which component they are, and which factory to use to render them. There is no secret there.

tony.kay18:10:00

The child should not know anything about the parent, because that would break things in unpredictable ways (the child can’t see all the parents that might use it)

tony.kay18:10:23

That’s why children give out well- known callbacks. It lets the parent control the known relationship

tony.kay18:10:11

So, while it is true changing a child query could break a parent, it is somewhat unlikely…and also relatively easy to debug

tony.kay18:10:11

the general contract of a component’s class is “I edit a person” for example…is name really going to disappear from that?

roklenarcic18:10:28

I guess. Here's a question, given this router: (ident [this props] [(-> props :modal :db/id) :singleton]) :warning-modal WarningModal :edit-modal PersonModal) Is it possible to have router ident vary in the second element of ident rather than the first one?

tony.kay18:10:10

nope…it can vary in both, but the “kind” has to be the first element of the ident. It’s how union queries work

roklenarcic18:10:27

Thought so. Thanks

tony.kay18:10:29

I know…kinda sucks when viewing app state

tony.kay18:10:58

Best you can hope for is naming your modal types in a more visible way…like :MODAL/WARNING

tony.kay18:10:13

then at least your eye is drawn to it

roklenarcic18:10:27

I also wish I could specify an ident as a transact! follow up read

tony.kay18:10:42

that you can do

tony.kay18:10:16

idents are allowed as follow-on reads

roklenarcic18:10:45

Really? Because I got error 2 is not ISequable when saying [:person/by-id 2] as a follow-up

tony.kay18:10:00

did you accidentally concat it?

tony.kay18:10:08

instead of conj

tony.kay18:10:19

sounds like it got flattened

roklenarcic18:10:30

let me check again

tony.kay18:10:44

(transact! this ['(f) [:person/by-id 2]]) should work fine

wilkerlucio18:10:54

cool, I didn't knew that 🙂

tony.kay18:10:27

yeah, it just looks up all components via the indexer’s :ref->components index

tony.kay18:10:28

a prop is looked up in the :prop->classes, and then :class->components

tony.kay18:10:39

resulting in a list of all components that need a refresh

tony.kay18:10:26

Then it sorts them by UI depth, and renders the ones closest to root first. If that covers some of the others (e.g. they are children), then that prevents the overhead of running extra queries and refreshes for them.

roklenarcic18:10:03

What's the easiest way to find out which component is lacking the react key that things complain about?

tony.kay18:10:53

two possibilities…the message itself, if read carefully, does tell you a lot. BUT, there is a little annoyance in Om Next’s internals that can cause that warning on built-in factories… @wilkerlucio could you remind us what that was?

tony.kay18:10:59

I’m drawing a blank

tony.kay18:10:21

an apply missing somewhere

roklenarcic18:10:22

I see a couple of :keyfn on things that don't get any props?

tony.kay18:10:50

that could do it…the message should tell you the UI path to the problem

roklenarcic18:10:15

I says that a div lacks it in a component that has a lot of divs 🙂

tony.kay18:10:31

It should only be a problem where you’ve passed things as a sequence

tony.kay18:10:38

e.g. passing an array as the children

roklenarcic18:10:29

Hm... (b/ui-modal-footer nil (b/button-group {:key "modal-btns"} (b/button {:kind :success} (tr "Save")) (b/button {:kind :warning} (tr "Cancel")))) Button group needed key or else warning

roklenarcic18:10:26

Yep... b/ui-modal-title, b/ui-modal-body, b/ui-modal-footer all require keys on their children, even if it's just one

tony.kay18:10:51

I seem to remember there was no way for me to work around that because of the Om Next issue…it’s somewhere in the chat history on here 😉

wilkerlucio19:10:14

@tony.kay the apply to accept children

wilkerlucio19:10:40

the problem with the current Om.next factory, is that it is sending the children as a list

wilkerlucio19:10:56

and if you taht with React, you have to specify the key for every element on the list

wilkerlucio19:10:03

this is ok for actual lists of items

wilkerlucio19:10:28

but it's annoying when you are just rendering a fixed list of elements (and react supports that without requiring key)

wilkerlucio19:10:55

the fix is to call apply on the js/React.createElement so the items are passed as positional items, and not a list