This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2017-11-01
Channels
- # aws (2)
- # aws-lambda (18)
- # beginners (68)
- # boot (6)
- # cider (2)
- # clara (2)
- # clojars (27)
- # clojure (68)
- # clojure-austin (5)
- # clojure-berlin (6)
- # clojure-dev (28)
- # clojure-greece (7)
- # clojure-italy (46)
- # clojure-japan (3)
- # clojure-nl (1)
- # clojure-russia (8)
- # clojure-sg (1)
- # clojure-spec (17)
- # clojure-uk (86)
- # clojurescript (82)
- # community-development (2)
- # cursive (18)
- # datomic (11)
- # duct (5)
- # fulcro (254)
- # garden (2)
- # graphql (6)
- # hoplon (19)
- # instaparse (4)
- # kekkonen (2)
- # leiningen (4)
- # luminus (3)
- # lumo (9)
- # off-topic (28)
- # om (7)
- # onyx (38)
- # other-languages (27)
- # portkey (7)
- # protorepl (1)
- # re-frame (56)
- # reagent (64)
- # ring (14)
- # ring-swagger (7)
- # shadow-cljs (255)
- # sql (2)
- # vim (11)
- # yada (10)
@tony.kay since you say it does not make sense for a component to not have an ident, does that mean we should only use functions (instead of sub-components) when we want to extract logic out of a component?
Also are you saying it doesn’t make sense for a component to not have an ident whenever you call transact!
on it? Or just in general all components should have an ident?
@currentoor I did mean stateful components (with a query)…not just plain-jane React components
@tony.kay Sounds really awesome. Since it sounds like a full re-write, any chance some changes will go in to make df/load, df/load-action work on the server for SSR ?
The current process is a bit manual. Keep thinking about how nice it would be when creating the application on the server, to just give it the fulcro-server-parser. All requests just go to the multimethods instead of http calls (no need for batching).
hello friends, I was looking at the fulcro-template
project
I noticed that the error message when you’re trying to login with bad credentials never appears
Looked at the app state and indeed, the :current-user
is nil even if attempt-login
returns it
tried to solve this one but did not find out
am I the only one with this bug ?
and offcourse /api
returns data so it’s more a merge/query bug
["^ ","~:current-user",["^ ","~:login/error","Invalid credentials"]]
@claudiu It isn’t a full rewrite at all. It is mostly done. It is a few internal changes to the Om Next internals that are not in line with Om Next’s design. The fact that we choose the default db format as the only db format are what enables them to be easily written.
But, I hear you on the load helpers for SSR. The specific thing you’re asking for is probably not easy, but something roughly equivalent and nice is possible
@baptiste-from-paris Hm. Yeah, I didn’t debug that bit of the template much…was just trying to show structure
yes offcourse, as I am learing fulcro, I don’t know where to look appart from app-state, query (have already some om project in prod)
but could it be a merge problem ?
I have a meeting right know, but I’ll look at it again in 2/3 hours after work
great, thanks a lot
and no broken dynamic queries with respect to history, time travel, and support viewer
@baptiste-from-paris So, the error message from the server needs to have a :uid for it to normalize…that’s why it is failing
Unfortunately, the UI code isn’t written to handle the case of a user whose login failed, so when I “fix it”, it lets you log in with any credential 😉
would never have found this one
so, with no uid
merge = nil ?
Fulcro adds in some other bits…one other problem was I didn’t include the :login/error in the User query…so that couldn’t go in state either…so you end up with trying to merge nothing at nowhere
I got this one
but the uid would have been hard to solve ^^
the only reason it got completely wiped out was that the map ended up empty because there was nothing that matched the query
nope it did not but I might have made it wrong, will look after this stupid meeting 😉
one thing that I noticed (also about the template) is the lack of href
on anchor elements
good for SEO on SSR
@tony.kay maybe Fulcro 2.0 is a good place to upgrade React to 16? what you think?
I’m not too worried about SEO on the template…you’re just going to completely change the UI anyway is my tought
@wilkerlucio yes. There is already a patch to do that on Om Next itself. I plan to pull it over
yes offcourse
yeah, had you tried it? I didn't had the chance yet
last I remember Antonio was fixing adv compilation issues
it has been merged
not sure if that was completed
yes I think so
sure, makes sense, I'm excited for Fulcro 2.0 🙂
and React 16, hehe
one thing that we should test is about the new possibility of retuning lists out of renders
probably will need a bit of conversion on the edges
would it break React using Fulcro components? E.g. If I define a component in Fulcro (without queries) and export it to js-land, can I use it if it returns a vector of things?
ok, could try it with uid and it’s workgin
yeah, the React 16 allows renders to return lists, so they can output multiple elements
Which is indeed logical because it’s waiting for a user
I think would be easy, just convert to JS array on the edge of the factory
I don’t know what’s the typical way to handle errors yet but I guess one way would be to have a union query to handles errors. As an error is not a User
@baptiste-from-paris for login I ended up just using a query, just df/load and in the post mutation I make the decision the response returns a error or a valid user :)
ok, thanks, nice 🙂
Here is the full patch for login problem (now on develop): https://github.com/fulcrologic/fulcro-template/commit/91a2a473c22cdfa59670e52f207340a7de0544b1
@baptiste-from-paris On this front I’m not going to be that picky….You got back a non-existing User
yes, it’s acceptable, you’re right
I guess there is the same problem with :server-down
have to check also
and I’d love to add some dynamic routing example to the project
So, that is part of the plan with 2.0…to give better abilities to deal with temporary outages. With working history nav and idempotent mutations we can retry operations automatically in this common case, and not have to write code at all (other than a single global “this has gone on too long” kind of way)
I need that for my project so I might have the time to add that if you’re okay
need which? server down? At the moment there is a hook for global network errors. I recommend just a modal overlay.
No, adding an example of dynamic routes to the template
Yes sorry
So, Dynamic Router is broken at the moment with respect to history. So, until 2.0 I don’t consider it a solid feature. However, the API won’t change. Not sure I want to add that much to the template. It is already quite large, and that complicates the build, etc.
I’d recommend you fork the template and publish it. Glad to add a link to it from docs
I want to keep what I inherit to maintain to a manageable level 🙂 It’s already a lot
ok, no problem
So, I think I just found a bug in Om Next…has anyone noticed that setting React component local state on a component that has a query re-runs the query against the parser?
@gardnervickers, I know you have performance concerns with your UI…have you seen this?
@mitchelkuijpers You just did DnD with setState, right? Did you notice the queries running for that component?
@tony.kay we don’t use component local state.
We do some things out of the regular transact loop using a :shared
and merge-state
to get performance.
but that swaps against the atom and causes a full UI refresh…I guess that’s faster than hitting db->tree
and path-meta
. Interesting. That confirms a suspicion I’ve had about larger apps: that the general optimization of shouldComponentUpdate is already quite good
With path-opt working correctly (Fulcro 2.0 fixes as well), you may find no need for your current tricks 🙂
Nice, we’re actually using pathopt in production, we have a shallow enough render tree we don’t hit any problems with it.
om/merge!
causes a full query?
If you hit the state atom, there is a watch on it that schedules a render. That kind of render will run a full query
We don’t mutate it, we initialize it on app mount so it’s working with our history viewer.
@wilkerlucio on React 16. You had the final suggestion for create class in Om issue 881. Is that all that remains?
@wilkerlucio It took me about an hour to convert our app over to pathom
last night and we were able to ditch about half of our server-side parsing logic plus replace our inconsistent version of ::p/entity
and ::p/path
which is fantastic. Really nice work!
@tony.kay I think was superseed by the changes Antonio did
@gardnervickers thanks for letting me know 🙂
are you driving a database directly, or connecting services?
@tony.kay just saw the issue about shared, you said it's not on history, do you think it should be part of the state?
I mean, it allows a function at the moment, so it can evolve over time, but nothing is snap-shotting what it was in the past
I’m also wanting to add the tx that caused the history state progression, so it might make more sense there
the cases I used shared were mostly to inject stateful shared stuff
so this worries me, if they get on the DB it will break things
like core-async channels, and other mutable stuff
when I want data, I just throw it at the app state
for what use cases are you thinking for shared?
I’m just saying, if you want to navigate to history point 5 steps ago, shared breaks it the way you’re using it
because I want contextualize by the app
I might have multiple fulcro apps running
yeah, I understand that…though you can get that using adv compiler option :ouput-wrapper
I just don't see why throw data on shared, if you can put it on the app state
maybe a wrong perception, but I always though shared as a place for stuff that I can't put on the app state
so, IMO should be a dev concern to ensure bad state on shared doesn't break the app somehow, not the FW
I always thought of shared as a place to put stuff that most components needed, but for which you didn’t want to write a link query
if you go that way, where would I throw those shared stateful things? that's my worry
I’m ok if you want to do something that is incompatible with serialized history…your choice. But I do want to make it work if you choose to make it work.
maybe make the "shared in state" optional
what is :output-wrapper
?
It tells Google Closure to wrap your entire app in a function, so it doesn’t pollute global
but that's on app level, I mean, when I was writing my blog to have live examples, in my own source I have multiple instances of fulcro app loaded
I have not personally tried it, but ran across it when thinking about the problem of Closure renaming crap in a way that accidentally collides with others, which is quite possible
the shared is the place I have to put context for each running fulcro client
if I move to a global, I would have to start filtering it somehow on my app, and lose the local context
ok, so in that use-case, support viewer simply cannot work. stateful breaks it no matter what
so, if you feel strong about having shared on state, maybe we have a flag to turn that off?
Why are you polluting your application with these stateful things outside of the network layer?
but those shared get on the network layer as well
I use then there
on mutations
sorry, no network, mutations
for example, if I want to have a component that stores local data, I can have an implementation using LocalStorage for the browser, and a differnet one for a Chrome Extension
The model of Om Next (and Fulcro) is that you have queries and mutations. Nothing else. The mutations are meant to do local optimistic updates and “remoting”. In my mind, “remoting” covers any side-effecting, async thing.
this is the kind of setup I like to do at client level, and currently the :shared
is the only place I can contextualize on that level
so, you could write your example as a “remote” that is implemented locally on the client. Now it is just catching that mutation or read
@tony.kay yes we noticed this too, we used react-set-state
@mitchelkuijpers ah, so you went around it 🙂
but I think writing as a remote is more complicated
@wilkerlucio Is it? It might be a little more code, but doesn’t it keep the core model super simple?
my main point is, currently :shared
is the only place I can have client-level stuff that doesn't have to be on the app-db
if we took that way, we lose that possibility
of having stateful stuff contextualized by client
I have another suggestion, maybe we have 2 different shared, one that does in the app db, other that doesn't
I just don't want to lose that option, I don't use it much, but I like knowing that if I need to use the option exists
So, I don’t think putting shared in app state is what I’d do either way. Too easy for collision, and I can hang it on the transition edge of history
ok, if it doesn't polute the app-data, I don't see any problems
sure, I understand that if I do that, I have to deal with the consequences, and I'm ok with that
I would encourage you (and anyone lurking) to consider the opinion that you should try your best to avoid anything in mutations on the client that side-effect beyond state. Using the “remote” mechanism pushes the more complex things (like async, etc) out to the layer that is designed for them. If you don’t care about support viewer and history navigation, then it matters less.
(transact! this [(set-local-value {:v 1})])
with (defmutation set-local-value [{:keys [v]}] (local-storage [env] true))
That thing gets queries and mutations, so you end up just using a parser to handle the requests….so, check out pathom
😄
The elegant simplicity of the query/mutation/immutable app state keeps your reasoning about things quite clear…AND having those ugly things at the “network” layer also means history (as I’m defining it) will have a record of why state changed from A to B (e.g. because mutation X or query Y returned)
@tony.kay we use shared for constants so we can also render server side. Then real globals get really annoying
We also have remotes for the HTML5 history and localstorage btw
@mitchelkuijpers Thanks for sharing. Never thought of remotes for those scenarios, sounds like a really nice & elegant solution.
@tony.kay sure, there are ways around it, ok, I'll try that, and I'll ping you if I ever get in a place that shared seems reasonable 😉
and a separated remote to that seems like a good idea
@wilkerlucio Wrt/ your previous question about our use of pathom, we’re doing joins between a bunch of data sources.
It really shines there, especially since Datomic is our main source of truth linking all our foreign keys.
when should I use om/set-state
instead of using state in app atom?
The intention of component local state is to handle fast-changing transient values, like DnD position and animation status
you don’t want the overhead of swapping on the app state, and don’t need that data in history
Ok, so not for determining if something is expanded or in edit mode or things like that?
reasonable to use for keystrokes in an input field, then move it to app state on blur
If I have a list of people, where I can click on one person, which then has detailed view which has list of transactions this person participates in, then if I click on transaction I get list of persons involved, and when I click on the person I get to same detailed view. How do I accomplish that and avoid problems with recursiveness of queries?
Ideally user can click on person->transaction->person->transaction ad infinitum, if they wanted to do so
It keeps swapping screens
not visually nested
right, so it isn’t a recursive query…you just change the idents to point to the next iteration, and the “nested” people are a different component that don’t list txes…then you shift state
I understand.
So, my tests of ILocalState seem to indicate that it is also buggy in Om Next. I’ve never seen even someone do an example of it (I had to read the source to see how it is supposed to work, and I see a bug as it stands that I can produce in a devcard). I’m kind of guessing no one in Om or Fulcro world is using it…if anyone thinks otherwise, I’d like to hear it. It’s another source of complexity I’m probably going to drop.
Also, I’m going to make set-state! be the same as react-set-state. The reasoning, as best I can tell, is to prevent react from updating the UI outside of the render animation loop, but unfortunately there is no way for the render loop to know not to run the query on the component, so IMHO it makes set-state! mostly useless for the high-performance transient tasks I think it should be used for.
Earlier you said you see no reason why every component should not use an ident. What would you use as an ident in a component that represents many-to-many relationship?
Or one to many
yes, should not
Yeah, but if I have a component that's a badge that shows how much a person spent in a particular transaction, I have basically two datoms: amount and person id. I can't make a good ident out of that
then it becomes part of the parent’s data, and you don’t have a component with a query
neither is unique and even the combination isn't. So I'd have to push down transaction id and have an ident of trn id, person id pair
oh...
just because you want to split out something with a defui doesn’t mean it has to be a stateful component
so I make a more complex query in parent component and just pass props down to render?
yeah that could work
yeah, don’t get over-zealous with the queries…if they don’t make sense for the data-driven aspect (e.g. have a unique identity on the server), then they are not buying you much
if you need to mutate it as a client-side stand-alone entity, then you want it to have an ident…that is a better simplification
So, I’m going back on what I said above. It is technically useful to have set-state!
be different than react-set-state!
So, the current implementation of set-state!
defers rendering until the next animation frame by offloading it to the reconcile stage (at which point queries are run) This prevents over-rendering (lots of set-state! calls causing many DOM updates), but forces the UI refresh to always take the overhead of a full reconcile for all queued components/keys. This makes set-state!
a friendlier citizen IF you’re using it for slow state storage, but means you have to revert to react-set-state
to get the really performant one. I had not realized this was there, and had always assumed set-state!
just wrapped React’s version. My personal philosophy is that set-state should refresh the component right then, but chains of set-states (e.g. child sets state, then does callback to parent that sets state) would then cause two forced refreshes at the actual DOM level.
This is another one where I feel like the added complexity of the internals might not be worth the outcome. Usage of set-state!
in general is already out-of-bounds for our reasoning model, and really should only ever be used for animation kinds of things and low-level DOM crap that cannot be represented via state (e.g. carrying around refs to components to do image manipulation).
I also see a technical issue where if you mixed calls to set-state and react-set-state you could screw things up
@mitchelkuijpers Thanks for sharing. Never thought of remotes for those scenarios, sounds like a really nice & elegant solution.