Fork me on GitHub
#untangled
<
2016-06-16
>
jasonjckn00:06:07

if Constructor will only have a single method initial-state

tony.kay00:06:09

now is the time for naming

jasonjckn00:06:11

then yes I would use InitialState

jasonjckn00:06:15

similarly IQuery and query

jasonjckn00:06:29

if you had more methods you want to add in the future

jasonjckn00:06:35

then maybe InitialState is too specific

jasonjckn00:06:03

IInitialState (initial-state ) would be the om.next naming convention

tony.kay00:06:26

Yeah, I'll give the union case a bit more thought. The problem is if I try to solve that then I have to WALK initial state, which is a pain. Ident does not have a leading I in Om...I think eliding it is ok

tony.kay00:06:26

k. headed out. g;night

ethangracer15:06:46

@tony.kay @jasonjckn my only hesitation with InitialState is how general it is. devs might confuse it with initLocalState, or with their preexisting understanding of React’s getInitialState. Maybe something like InitialAppState? Or InitialGlobalState?

tony.kay15:06:46

Untangled Spec 0.3.7-1 Released to Clojars

tony.kay15:06:06

0.3.7 has a slight transitive dependency issue

tony.kay17:06:57

easy to forget that possible confusion when we're always pushing people away from local state.

tony.kay17:06:53

Thing is, I'm planning on using it for a bit more than just initial app state (though that is the primary use)

tony.kay17:06:52

The new merge-state! function (just pushed an update to clojars 0.5.3-SNAPSHOT if anyone wants to play) is experimentally using these constructors to establish a sane initial state for merge to target in cases where the object wasn't already in app state.

tony.kay17:06:27

This is useful in cases where things like :ui/attr or empty lists need an initial value for correct initial rendering

tony.kay17:06:00

I'm toying with making it optional..but I'm not sure if the default should be that it uses it and you have to turn it off, or that it is available if you turn it on.

tony.kay17:06:55

Given that the constructor accepts parameters, I'm thinking "off by default", and "on if you pass constructor parameters" is the correct thing to do

tony.kay17:06:58

One place this was biting us in server push was in a case where a child had no props in state, but was doing a link join...e.g. [{[:thing '_] [:data]}]. If you construct app state so that the parent's join of this query goes through "nothing", then the child query doesn't even run...meaning you need to at least plop an empty map into app state where the child lives.

ethangracer17:06:54

Hm. I think off by default is probably the way to go at least to start. App-state can be pretty finicky, wouldn’t want to accidentally corrupt data.

tony.kay17:06:58

constructor is good for this use-case. But then it occurred to me that if your default :ui/checked or something was true you'd have a similar problem

ethangracer17:06:18

But I see what you mean

tony.kay17:06:30

hm. Is there any way it would corrupt data?

ethangracer17:06:45

I can’t think of it off the top of my head, no

tony.kay17:06:59

erm, I guess I can

ethangracer17:06:03

Just had enough times that I thought modifying the app state would be harmless and turned out… not so much

tony.kay17:06:27

we're composing these things recursively, so you might end up with some children states you didn't intend...but that is my reasoning for taking parameters in the constructor.

tony.kay17:06:45

which is still the "off by default"

tony.kay17:06:22

but then you're explicitly calling merge-state!, and it would be documented

ethangracer17:06:23

so what to call it then? it is still kind of initial app state if it hasn’t existed before

tony.kay17:06:47

It is a constructor...there is no confusion there. It is used when you build a new component.

tony.kay17:06:04

The idea is that the app state of it can evolve or be injected from a remote query

ethangracer17:06:46

even when the component has already been constructed?

tony.kay17:06:00

The only reason I'm considering using it in merge-state! is that the function is meant to merge a new object into app state.

tony.kay17:06:03

from a server push

tony.kay17:06:22

A mutation on an object would be starting from the initial constructed state

tony.kay17:06:40

so, the mutation would assume things had started out a certain way

tony.kay17:06:15

this causes a subtle difference between how initial affects progression and how merging an object from the server might act

tony.kay17:06:47

Similar difference when doing mutations that create new things...but there is nothing that keeps you from explicitly calling these constructors to get your initial state (and I continnue to consider that your responsibility)

tony.kay17:06:53

Then again, I could be convinced that these are just for initial app state, and you should do your own thing if you want some UI concern injected into your objects as you create/merge them into app state

tony.kay17:06:22

which goes back to your suggestion of calling it initial app state

tony.kay17:06:14

@jasonjckn: @currentoor @therabidbanana Be interested in your input on this...kind of a possible "big deal" 🙂

jasonjckn17:06:56

side note on semantics I consider "constructors" to mean constructing an instance of a class, e.g. it would be called every time the factory is called. initLocalState is much closer to a 'constructor' than this function

tony.kay17:06:51

@jasonjckn: Sort of...constructors initialize state, which is what this is doing. Factory in React creates a on-screen representation instance that just renders state

tony.kay17:06:14

the state is in the app, and for all intents and purposes represents what there is of the "instance"

jasonjckn17:06:25

yah it's like the constructor on the app

jasonjckn17:06:35

except in fragments across the UI hieracrhy

tony.kay17:06:58

I would argue it is the constructor of the instance, and the fact that that is stored in a larger map is an artifact of implementation of the framework

tony.kay17:06:18

but then queries blur the line as well

tony.kay17:06:33

because I can pull that state into other on-screen instances

tony.kay17:06:13

I'm moving very rapidly to the view that it is initial app state after making that statement 🙂

tony.kay17:06:32

an artifact of startup only (convenience)

therabidbanana17:06:53

Not sure I fully understand what we're talking about here - is there a good cookbook example with these new constructor things to take a look at?

jasonjckn17:06:58

I wish it was merely an artifact of the implementation, if i didn't have to think about the implementation at all that would be wonderful, but at the moment certain Constructor's like Tabs in a TabUnion aren't called( without much fore thought)

tony.kay17:06:08

not in the cookbook, but I did make a thing you can play with.

tony.kay17:06:28

@therabidbanana: It's a way of being able to think about initial app state in the same context as queries...simplifies startup code quite a bit.

tony.kay17:06:41

but is tempting to use to initialize things on server push as well

tony.kay17:06:59

which is what led to this discussion...are they constructors on simply start-up helpers....or something else?

tony.kay17:06:18

e.g. the fact that they accept parameters means you could leverage them in interesting ways in your mutations

tony.kay17:06:39

(e.g. as constructors for new local instances)

tony.kay17:06:15

all of those use-cases could be done with your own interfaces as well that you place on your component as static

jasonjckn17:06:35

is there a way to use the query fragments to automatically call constructors all the way down the tree

tony.kay17:06:45

overloading it based on parameters is probably a bad idea

mahinshaw17:06:50

I think exposing access to the constructor would be nice, but not backing any functionality in with them. I agree that merging data would be potentially simpler in cases where you have the constructors params/values. But that should be a tool for the implementer

tony.kay17:06:53

@jasonjckn: No need...you compose them like queries

jasonjckn17:06:22

the current implementation make initial-state feel more like a helper function because only one of your children gets constructed in a TabUnion. Whereas if the construction always happened unconditionally, it's simpler + i like the work constructor a lot more now

tony.kay17:06:42

Oh, you were asking about the union case.

jasonjckn17:06:47

hence suggesting to use query fragments to help do construction

jasonjckn17:06:06

in this new version of the API the initial-state wouldn't be recursive anymore

tony.kay17:06:18

So, at the moment I'm still sticking with started-callback to initialize union alternates

jasonjckn17:06:18

you would walk the query fragments to get that recursion

jasonjckn17:06:43

ok, food for thought anyways

tony.kay17:06:51

But I do understand what you're saying...it is a possible avenue to explore

tony.kay17:06:04

I like the explicit inclusion...less voodoo

tony.kay17:06:40

If we made an untangled/get-initial-state function and used that for composition (like om/get-query) then we could handle the union case automatically, I think

tony.kay17:06:47

which would be pretty sweet

tony.kay17:06:54

I'll probably work on that today

tony.kay18:06:25

So, at the moment, I'm leaning towards the suggested name InitialAppState for the protocol.

tony.kay18:06:45

Not sure what to do for the merge-state! cases from above...having a "constructor of new base state" is going to be pretty useful

tony.kay18:06:50

I mean, users can certainly do (merge {:ui/checked true} incoming-object) and then pass that to merge-state!...but it isn't very good local reasoning in the context of a component

tony.kay18:06:33

I guess them implementing their own protocol could fix that to: (merge (get-base-state Component) incoming-comp-data)

tony.kay18:06:05

but it is trivial to make merge-state! look for the interface and do that for you.

tony.kay18:06:43

then again, people can write a little helper function wrapper for just that

tony.kay18:06:53

I think leaving that bit to the user is probably better

tony.kay18:06:51

much less invasive, and leaves flexibility open...just doesn't feel as refined. Library design is always hard.

jasonjckn18:06:43

as much as om.next left a lot to be included, i still like how flexible it is by just giving you the building blocks

jasonjckn18:06:51

so you can continue that same philosophy

tony.kay18:06:08

Right, but we're up a level. Framework instead of library.

jasonjckn18:06:19

nods tough call

tony.kay18:06:28

batteries are intended to be included, just not overbearing 🙂

jasonjckn18:06:50

what about a lifecycle function onAppStarted for each component?

jasonjckn18:06:04

that could be useful, for example calling merge-state! for your tab in a tab union 😛

tony.kay18:06:29

I think I've got the union thing solved...a lifecycle sounds like a big can of worms for misuse

jasonjckn18:06:36

anyways if you ever add that, you would want AppConstructor (initial-state) (on-app-started) or some protocol

tony.kay18:06:11

It isn't really workable. The React onComponentMounted is the best you could do, and it would be identical...but chicken and egg problems

tony.kay18:06:20

can't mount until you have data, but no data until you mount

tony.kay18:06:20

otherwise you have to scan the code for everything that is a defui component and run it's stuff...even if it isn't composed in in ways you understand (to-many, to-one?).

tony.kay18:06:33

yeah...messy and hard to reason about

jasonjckn18:06:41

ok, well for the implementation, I would just walk the query tree , and call on-app-started 🙂

jasonjckn18:06:47

some idea as 10 minutes ago

tony.kay18:06:55

oh, from Root you mean

tony.kay18:06:03

the query isn't enough

tony.kay18:06:10

ambiguity on to-many and to-one

tony.kay18:06:17

you need the data to resolve, but you don't have the data until you've called the callback...I don't know, maybe you're seeing something I'm not

tony.kay18:06:18

I guess if you worked through all of the possible combinations and cases you could prove whether or not it could work. I'm not personally up for that at the moment, but I'd be game to look at such an analysis

jasonjckn18:06:39

ok, i mostly brought that up to consider AppConstructor variants

jasonjckn18:06:51

if we consider that a possible future

jasonjckn18:06:24

anyways +1 for InitialAppState

tony.kay18:06:46

The incremental steps I'm considering are: 1. Initial app state: I think we've solved this one 2. Some kind of "when making a new instance of this component in app state, here's stuff to include that won't come in from a server, like a checkbox state or child placeholder to ensure the child query is evaluated"

tony.kay18:06:30

Those are the two specific problems I'm solving

therabidbanana18:06:00

We've been relying on initLocalState for #2 mostly, though I know it's not ideal.

tony.kay18:06:00

back in a few...talk amongst yourselves...

jasonjckn18:06:10

are we saying #1 and #2 must be different functions?

jasonjckn18:06:17

or that's up for discussion too

tony.kay18:06:25

@therabidbanana: But if you use that, you don't get support viewer and good debug support

tony.kay18:06:47

@jasonjckn: I was (earlier) suggesting that (1) could be used for (2), but I'm thinking now that that is a bad idea

tony.kay18:06:55

at least if it is part of the framework

tony.kay18:06:39

(2) is easily solved yourself, as I said a bit ago with all of that merge speak 🙂

jasonjckn18:06:50

you had a use case where it was bad ? sorry i missed it

tony.kay18:06:35

Intial app state has children, which have initial state...some new thing coming in won't necessarily share that vision of what that subgraph should look like

tony.kay18:06:11

In many cases, though, it will be the same

tony.kay18:06:20

which was why I was confused about it myself

therabidbanana18:06:34

I do like the support viewer and want to use it at some point, but we decided that for modals with forms where you might be changing state or canceling to revert, it was easier to go with local state. We might refactor at some point, but basically doing a transaction for every letter typed seemed like a bit heavy-handed solution (maybe we were doing something wrong there)

jasonjckn18:06:44

i wonder if we can make the default case in this framework where it's the same function for #1 # 2, but not preclude the user making #1 and #2 distinct

jasonjckn18:06:53

i think #1 and #2 are often the same for many apps

jasonjckn18:06:58

same function*

tony.kay18:06:17

@therabidbanana: using component-local state for form field input is often what we do as well...for just that reason

tony.kay18:06:30

it's the exception to the rule

tony.kay18:06:25

@jasonjckn: not really about if we can 😉 About if we should

therabidbanana18:06:44

Okay, cool. It's the only case we rely on local state anyway, but that's really the only time we need extra ui/type fields to be initialized anyway - when editing data.

jasonjckn18:06:48

should we? 😉

therabidbanana18:06:18

So for us the #2 case hasn't really happened outside of places where initLocalState isn't involved

jasonjckn18:06:43

i don't have enough use cases in my mind though, you're definitely more experienced than me, if 80% of apps have the same function for #1 and #2, then i think that should be the default (same function for #1 and #2)

tony.kay18:06:59

The (2) case is rare here as well...mainly comes up when a child has only a link query...not any data of its own

tony.kay18:06:30

@jasonjckn: 80%++ of the cases don't need (2) at all

tony.kay18:06:37

so perhaps that is enough to say "don't bother"

jasonjckn18:06:46

or at least hold off for now

tony.kay18:06:24

OK. I'm going to see if I can make the union case better for (1), and ignore (2)

therabidbanana18:06:45

On a mostly unrelated note - I'm to a feature I'm not sure how best to handle within untangled - autocomplete search. Has anyone here had to implement something like that/thought about how to?

tony.kay18:06:48

as in, periodically hit the server for suggestions?

therabidbanana18:06:27

And the server is going to talk to a slow third-party to generate the results - 1+ seconds will be the norm

tony.kay18:06:37

so, remember that your renderer should be thought of as a pure function...you hand it data, it renders it. So your component will need state to represent exactly how it should look in all possible states. That is step one.

tony.kay18:06:16

Step 2 is to set up a mutation that can trigger your external API request...typically through XHRIO or something...with callbacks and that whole mess

tony.kay18:06:42

you'll need state to track in progress, etc...ok to close over app state and swap! on app state in the callbacks

tony.kay18:06:57

on return, you'll want the new merge-state! function 🙂

therabidbanana18:06:09

Heh, yeah, that's why I said "mostly unrelated". 😄

tony.kay18:06:18

you can do it with om/merge

tony.kay18:06:28

in either case you'll need to have access to the app or reconciler

tony.kay18:06:46

because this is "side-band" data

tony.kay18:06:20

of course, debouncing, not more than one running at once, etc.

therabidbanana18:06:41

For most of our selects we have this already: https://github.com/JedWatson/react-select - if we gave Select.Async a "loadOptions" callback we could probably circumvent mutations completely and rely solely on the local state of that component - it already does the debouncing, caching of results, etc

therabidbanana18:06:56

But that approach seems less than ideal.

tony.kay18:06:56

you can do that

tony.kay19:06:29

local state is fast and easy

tony.kay19:06:01

I have not personally tried integrating external components, but I see no reason why it would not work...

tony.kay19:06:16

maybe there are some re-render issues I'm not seeing, but can't speak to stuff I don't see 🙂

therabidbanana19:06:45

I'll give that approach a shot first, seems like the easiest way to deliver something, though potentially having these results in app state could be nice

therabidbanana19:06:44

(For our reporting project, we need to go and fetch a list of things you could filter the report on - if that list was cached somewhere in app state creating multiple widgets with same filters would potentially be quicker)

tony.kay19:06:07

also more visible for debugging, etc

tony.kay19:06:02

It actually gets a lot better if you can do it via a remote query via load-data

tony.kay19:06:21

e.g. your Untangled server does the comm to 3rd party, which you may be saying

therabidbanana19:06:39

Yeah, that's what I'm debating between essentially

therabidbanana19:06:00

Go within the system we're already familiar with, or set up an entire sideband approach

tony.kay19:06:01

In that case you could just shove off a load-data...the debouncing gets a bit more delicate

tony.kay19:06:30

you might want to put a flag in app state that you set/unset when one is in progress, and use :parallel true on the load

tony.kay19:06:45

the response get's merged, do a post-mutation to unset the flag

tony.kay19:06:50

kinda nice, actually

jasonjckn19:06:02

for untangled-server: how do I choose the port 3001 using environment variables, i don't want to use a config file

tony.kay19:06:06

takes care of refresh nicely

tony.kay19:06:29

@jasonjckn: use more than one config file, and use -DconfigFile 😉

jasonjckn19:06:44

then I need to generate config files using environment variables as the template parameters

therabidbanana19:06:55

We use environ in our project

jasonjckn19:06:08

would be better if the api let me just pass a map of config

jasonjckn19:06:17

if I wanted to slurp it, that's my choice

therabidbanana19:06:18

+ a custom config component

tony.kay19:06:28

YOu can override config, I'm pretty sure

tony.kay19:06:38

they are just SS components

tony.kay19:06:00

Our default is meant for our production requirements: That you can't accidentally start something that isn't configured by the admin of the production system.

tony.kay19:06:37

I'd be ok extending the config component to support a config file that references env variables, though, if anyone wants to contribute

jasonjckn19:06:37

sure that's a nice idea, but it's too restrictive, we have our own cluster setup and requirements

jasonjckn19:06:10

i will generate the config files if I need to

jasonjckn19:06:14

but extra step 🙂

jasonjckn19:06:21

would rather just read directly from ENV

therabidbanana19:06:32

^ we use something like this

jasonjckn19:06:40

thanks! 🙂

therabidbanana19:06:46

To boot the untangled system with a custom config component

therabidbanana19:06:20

Maybe we should add a cookbook for it?

tony.kay19:06:40

Definitely not opposed 🙂 Paired down of course

jasonjckn19:06:06

@therabidbanana: so that code sets the untangled-server port right?

therabidbanana19:06:46

Yeah, port is the http port (8000 here, configurable via ENV[HTTP_PORT]). As long as untangled when it boots can find the port number at (get-in system [:config :value :port]) it'll be able to do the right thing.

jasonjckn19:06:03

ok that was pretty straightforward, so i'm less bothered by :config-path now

jasonjckn19:06:18

didn't know about :config :value

tony.kay19:06:49

config is the component. Value is the value of the configuration data within it

tony.kay19:06:00

avoids arbitrary names within the config from accidentally overwriting meta info in the config component itself

tony.kay19:06:10

(if you were to merge to two together)

adambrosio19:06:26

so in: https://github.com/untangled-web/untangled-server/commit/c9d07f8b0f4582b210a99f80e3923866a1275e92 & https://github.com/untangled-web/untangled-server/commit/3f2b61cbb22f944ca0df7eab52c409dd0fa9e090 i made it so you can reference environmental variables by having map values in the config be keywords with the namespace :env/ENV or :env.edn/ENV and it will look for them in System/getenv

adambrosio19:06:42

the difference between env and env.edn is that i do a clojure.edn/read-string on env.edn

tony.kay19:06:45

Oh, nice...I missed that 🙂

adambrosio19:06:52

yeah it kinda sneaked it

adambrosio19:06:30

oh wow it really sneaked it, forgot to mention it in the changelog

adambrosio19:06:39

since i wrote the config component, let me know if you need more or something changed

tony.kay19:06:23

So, I've renamed Constructor to InitialAppState and pushed SNAPSHOT to clojars (updated training-2 as well)

tony.kay19:06:49

None of the ideas for union initialization are working out...I'm still recommending merge-state for setting up those at startup

jasonjckn20:06:45

@tony.kay: what if we don't need every tab's app state initialized during application start-up, the problem we're solving is just default values for the app state, so if you have some API for getting the base state of a Tab, it'll be called when the user selects that tab, and with this we don't need to manually call merge-state! anymore

jasonjckn20:06:26

so the app state for the tab would get initialized automagically when choose-tab mutation is executed and untangled sees no default values

jasonjckn20:06:31

i'll let you fill in the blanks 🙂

tony.kay20:06:08

yes, but then you have to make extra logic in your mutation to see if the base stuff is there...and many tabs will have base state

tony.kay20:06:15

I had considered this

jasonjckn20:06:31

ok, i thought maybe there's some logic you can write once in untangled for this, obviously i don't want to write this logic in the mutation

tony.kay20:06:47

It is certainly something you can already do, but if you want your tabs to work out of the box with a simple mutation on an ident it would be nice to have them just work

tony.kay20:06:08

I think I've got the solution. Here is what I'm doing:

tony.kay20:06:30

For union components, IF you supply initial state, it MUST be a map with a default key

tony.kay20:06:39

(the default indicates which branch is the default)

tony.kay20:06:57

hmmm...to-many is still half-baked....anyway, for to-one you do this

tony.kay20:06:17

{:main (initial-state Main) :settings (initial-state Settings) :default :settings}

tony.kay20:06:01

The you use a helper function (get-initial-state) instead of initial-state directly....so actually, it loks like this:

tony.kay20:06:22

{:main (get-initial-state Main) :settings (get-initial-state Settings) :default :settings}

tony.kay20:06:51

The get-initial-state helper detects the union case, and returns just the default branch (e.g. is would return the state for just Settings in the example)

tony.kay20:06:02

it hangs the others in state as metadata

tony.kay20:06:19

Then, Untangled during startup can scan app state for this metadata and finish initializing them

jasonjckn20:06:41

sure i thought about that too

jasonjckn20:06:59

it's not a very simple solution

tony.kay20:06:12

what do you mean? From the user's perspective it is trivial

jasonjckn20:06:14

i like the current construct at least it's simple

tony.kay20:06:26

Untangled handles the work

tony.kay20:06:49

nothing is exposed, and the map matches the union map query

jasonjckn20:06:12

yah, i'm open to it, personally i try to avoid adding new concepts where possible, with the current API at least initial-state always returns object data in every case

jasonjckn20:06:25

now you're adding a new type in a sense, the return value is object data OR special union data type

tony.kay20:06:48

Unions are already an outlier that doesn't match the patterns (it has not state of its own)...this actually brings it more inline

jasonjckn20:06:29

yah.. it's a decent solution

jasonjckn20:06:18

i feel like there's more we could do along the lines I was talking about above, but that's the only other contender for this problem I think

jasonjckn20:06:34

"but then you have to make extra logic in your mutation to see if the base stuff is there...and many tabs will have base state" when you write code like (update-in [:current-tab 0] ...) it's short and sweet to update an ident, but i think this general pattern of checking for unitialized data could be quite powerful

jasonjckn20:06:05

this logic would come up again and again , you want to set an ident, and that object doesn't yet exist

tony.kay20:06:56

yep. It is an initial state problem. Some of those you do solve in mutations (or remote queries)..but this one seems a common one where you know what you want to be there from the start

jasonjckn20:06:42

i think both things could be in untangled, your union fix, and some faciilty for automatically initializing state when the entity doesn't exist yet

tony.kay20:06:15

I'll let you think about the latter...proposals are welcome...I'm going to keep my brain on task at hand

jasonjckn20:06:31

since you're adding magic, consider also just walking the Query to identify union case, here you're adding in meta data and walking the object-initial state structure

jasonjckn20:06:02

if you walked the query you wouldn't need these new concepts get-initial-state and

{:main (get-initial-state Main) :settings (get-initial-state Settings) :default :settings}
nothing would change in the Constructor, the only difference is you would see union cases and call merge-state for the other tabs

tony.kay20:06:43

Think it through...Om does the query walk and normalization..I'd have to rewrite that or hack into it

tony.kay20:06:49

neither seems a good idea

tony.kay20:06:03

remember we're normalizing all this crap 🙂

jasonjckn20:06:20

maybe I don't understand, the walk seems pretty simple to me, postwalk the Root query, then locate all the unions, look at the OM meta data to grab the component used for the union and call merge-state!

tony.kay20:06:24

Go read Om tree->db...you're suggesting I rewrite that whole thing as far as I can tell..I see no way to do what you're saying otherwise

jasonjckn20:06:50

call initial-state for each component in the union and merge-state! the object data

tony.kay20:06:41

So, can you assemble an example in the training-2 demo?

jasonjckn20:06:08

you want me to implement the walk function?

jasonjckn20:06:25

i'd love to be i need to get some stuff done today actually

jasonjckn20:06:56

i could have it done by next week

jasonjckn20:06:08

but if that's outside your timeline, i trust your solution will be fine too

tony.kay20:06:10

I see what you're saying, I think

tony.kay20:06:46

The queries are not trivial to walk, but easy enough to turn into an AST

tony.kay20:06:51

(Om has functions)

tony.kay20:06:09

Pull out the unions

tony.kay20:06:16

I think the Components are on the AST...don't remember

jasonjckn20:06:22

i believe so too

tony.kay20:06:48

They have to have idents

tony.kay20:06:16

so, then you don't have InitialAppState on the Union component at all..just the sub-components you want initialized

jasonjckn20:06:23

if you go this route than the Constructors never change 😄 they still just return (initial-state TabA nil)

tony.kay20:06:26

oh wait..you still need to know which is the "default"

tony.kay20:06:41

yeah, that could work

jasonjckn20:06:45

yes the constructor of the TabUnion tells you the default

tony.kay20:06:09

ok. Let me play with that. Might be tractable.

tony.kay20:06:32

also need to figure out the to-one to-many difference

tony.kay20:06:49

I guess if the union initial-state return is a vector (to-many), then you skip this whole initialization for it

tony.kay20:06:23

yeah, I think your idea might be better.

jasonjckn21:06:06

walking the AST should be straightforward (defmethod walkfn :default [] (recurse on children) ) (defmethod walkfn :union [] .... logic ... (recurse on children ))

jasonjckn21:06:23

or just use postwalk

tony.kay21:06:59

The AST does have the components...so yeah, it is pretty tractable

tony.kay21:06:38

The AST doesn't include the component for the union itself

tony.kay21:06:43

for some reason

tony.kay21:06:01

I wonder if that is a bug in Om

tony.kay21:06:20

oh...it is up at the join

tony.kay21:06:16

@jasonjckn: Hm. As I suspected. I see no way to resolve the to-many to-one differences

jasonjckn21:06:30

ok, didn't that apply to both solutions?

jasonjckn21:06:39

are we still going ahead with walking the AST solution?

tony.kay21:06:58

Yes, walking the AST gets me the list of things to do for the to-one case, but that will be wrong in the to-many

tony.kay21:06:13

The query doesn't say which it is...the info just isn't there

tony.kay21:06:20

(it cannot be)

jasonjckn21:06:39

ok you had some idea about initial-state returning a vector for the to-many case

jasonjckn21:06:57

you need to call initial state in the AST walk regardless to see which of the Tab is going to be initialized by default

tony.kay21:06:07

oh wait...you're right...I had the idea and forgot it

tony.kay21:06:23

let me see if that works

tony.kay21:06:38

it is a bit of a pain, because the union component itself is in the AST as a join...containing a union and union-entries

jasonjckn21:06:02

sure, worth it though, i like this API

tony.kay21:06:31

ok, what do we do if they don't supply an INitialAppState for the Union component itself...then we lose the detection

tony.kay21:06:45

but they may not want initialization, so it isn't an error per se

jasonjckn21:06:00

yes, i think that communicates to not initialize

tony.kay21:06:00

but they may supply InitialAppState on the sub-components

tony.kay21:06:22

it is mysterious...I guess I'll add a log warning (which will not show in production)

tony.kay21:06:47

easy to make a mistake you don't understand and beat your head against for hours

tony.kay21:06:22

Yeah, that'll work...only show the warning if the union children have initial app state but it is ignored because you forgot the intermediate

therabidbanana22:06:53

So trying to do this search through untangled as much as possible - so far I have - does it seem reasonable? 0. app state has :search-results/accounts {:results [] }, :search-queries {} 1. when search input is typed in, debounce: #(om/transact this [(search/query {:term %}]) 2. search/query would run load-data-action for [:search-queries term] with a parallel post-mutation search/merge-results 3. read server-side makes actual request and returns data eventually 4. search/merge-results pulls queries out of some holder variable and puts them back into :search-results/accounts :results (merge and run distinct on existing results so we can search on results we've already pulled)

therabidbanana22:06:11

Alternative solution of skipping past untangled - I'd probably still want to be able to send a standard Om read to the endpoint, is it doable to get a hold of the networking code from the untangled client and make a request with it?

tony.kay22:06:57

so in (1) you can put a marker that says you're in progress, which in turn can cause that mutation to become a no op while present

tony.kay22:06:10

which is removed in post-mutation of (2)

tony.kay22:06:40

combined with timed debounce, if you want...

tony.kay22:06:00

otherwise sounds right

tony.kay22:06:58

If you're skipping past Untangled, you should make your own request. Doesn't make sense to mix/match...the networking code would get too complicated trying to track what your side-band thing is doing...just use Closure XHRIO or something

tony.kay22:06:08

as I described earlier

therabidbanana22:06:41

Thought that would probably be the answer - we have stuff already set up for authentication and all that I'd have to redo going outside untangled, so hoping not to have to re-roll that. I'll see how the first approach works out.

tony.kay22:06:21

FYI, automatic union initialization pushed on 0.5.3-SNAPSHOT...training-2 example updated.

tony.kay22:06:37

There is a small bug where it adds some cruft, but that doesn't hurt anything..just an annoyance when viewing app state

tony.kay22:06:53

@jasonjckn: That worked out. If you're using that stuff, I'd appreciate a trial run

jasonjckn22:06:11

yah no sweat

jasonjckn22:06:16

i'll do it later today

tony.kay22:06:39

no more manual merge-state! needed, and it auto-detects to-many and does nothing in that case...also got the warning in if you've forgotten the default state

jasonjckn22:06:07

for testing i'll need a SNAPSHOT

jasonjckn23:06:22

@tony.kay: So i upgraded from 0.5.2 to 0.5.3 snapshot, changed Constructor to InitialAppState and took out the merge-state! for the tabs, when I load my app here's my app-state

jasonjckn23:06:24

:messages
 {:tab
  {:id :tab,
   :which-tab :messages,
   :results {:page-size 10, :type "message"},
   [:listings [:which-tab]]
   :untangled.client.impl.om-plumbing/not-found,
   [:messages
    [:which-tab {:results [:page-size :offset :items :query]}]]
   :untangled.client.impl.om-plumbing/not-found,
   [:settings [:which-tab :settings-content]]
   :untangled.client.impl.om-plumbing/not-found}},
 :listings
 {:tab
  {:id :tab,
   :which-tab :listings,
   [:listings [:which-tab]]
   :untangled.client.impl.om-plumbing/not-found,
   [:messages
    [:which-tab {:results [:page-size :offset :items :query]}]]
   :untangled.client.impl.om-plumbing/not-found,
   [:settings [:which-tab :settings-content]]
   :untangled.client.impl.om-plumbing/not-found}},
 :om.next/tables #{},
 :settings
 {:tab
  {:id :tab,
   :which-tab :settings,
   [:listings [:which-tab]]
   :untangled.client.impl.om-plumbing/not-found,
   [:messages
    [:which-tab {:results [:page-size :offset :items :query]}]]
   :untangled.client.impl.om-plumbing/not-found,
   [:settings [:which-tab :settings-content]]
   :untangled.client.impl.om-plumbing/not-found}}}

tony.kay23:06:50

you worried about the not-found stuff?

tony.kay23:06:11

you can ignore it...bug with a plumbing thing...just extra cruft

tony.kay23:06:23

I'll try to get that fixed soon...but it should not interfere with things

tony.kay23:06:30

(other than it being clutter)

jasonjckn23:06:37

let me test i think it works other than that

jasonjckn23:06:53

yup confirmed

jasonjckn23:06:02

app is fully working

tony.kay23:06:17

thanks for pushing on the idea

jasonjckn23:06:20

even less code, i like that 😄

jasonjckn23:06:25

thanks for implementing!

tony.kay23:06:30

let's you know you're on the right track

tony.kay23:06:37

when the code base shrinks

jasonjckn23:06:17

@tony.kay: actually i see some unnormalized data

jasonjckn23:06:41

my default tab has some unnormalized data

jasonjckn23:06:51

(defui MessagesTab [:which-tab
                    {:results (get-query SearchResults)}]
  static InitialAppState
  (initial-state [_ _]
    {:id :tab
     :which-tab :messages
     :results (initial-state SearchResults nil)})

  Object
  (render [T]
    (let [{:keys [results]} (props T)]
      (div
       (@SearchResults results)))))

jasonjckn23:06:00

the :results is unnormalized

jasonjckn23:06:29

more code relevant:

jasonjckn23:06:30

(defui TabUnion {:listings (get-query ListingsTab)
                 :messages (get-query MessagesTab)
                 :settings (get-query SettingsTab)}
  static Ident
  (ident [T {:keys [which-tab]}] [which-tab :tab])

  static InitialAppState
  (initial-state [_ _] (initial-state MessagesTab nil))

jasonjckn23:06:33

i'll double check it was normalized before upgrading, but i'm 99% sure it was

tony.kay23:06:36

Hm. It is possible I screwed up on extracting the query....if so, I'll fix it on Monday...I'm kinda done today 🙂

tony.kay23:06:45

or you can submit a PR if you see the problem

tony.kay23:06:02

I just made a to-one/to-many example in cookbook, and that is working

tony.kay23:06:15

on the develop branch

tony.kay23:06:27

but I didn't look too closely at normalization

tony.kay23:06:35

was just trusting Om

tony.kay23:06:49

Your example you sent is missing IQuery interface

jasonjckn23:06:57

yup i wrote a macro lol

tony.kay23:06:58

so it has no query

jasonjckn23:06:01

defui is my macro

jasonjckn23:06:17

i just downgraded, and confirmed that normalization is happening again

tony.kay23:06:27

ah, even with your macro?

jasonjckn23:06:39

my macro just adds in the IQuery for you

tony.kay23:06:29

Hm...not seeing why...the new code is in untangled.client.core. Roughly line 258

tony.kay23:06:47

The resulting merge-state! call should be identical to what was hapenning before

tony.kay23:06:05

perhaps query->ast lost some metadata

tony.kay23:06:42

well, anyhow...I'll look at it later

tony.kay23:06:38

I just pushed a bump on clojars, in case I spaced pushing some change and you have a bad snapshot

jasonjckn23:06:38

i did another test with latest clojars, bug still present