untangled-web / untangled-client Goto Github PK

Use case: I have no need in my app to show "loading.." in the UI i just want the old data rendered until the new data comes in

Reports and dashboards can spin off things that take a long time to process on the server. Since these are not related to mutation it is entirely possible to run these in parallel (our default is sequential for reasoning). We'd also like there to not be a network connection that can time out waiting for the response when computing something large.

Additionally, the server-side concern is that these intense computations should be bounded somehow (so even when done in parallel we can control how many run at once on a given server).

So, it makes sense to create a feature as follows:

• The ability to specify loads that should happen "in the background" on the server
• Such operations are sent via the normal sequential queue, but respond immediately with a special value that causes Untangled to watch (e.g. poll) for the later result (holding onto the query for that future merge).
• The server puts such operations into a thread pool via futures, and tracks completion. The result is cached (for a configurable amount of time) for response to the polling operation.

Independent of the network layer (websocket or http) we would probably just poll for the result, which would simplify the implementation across any pluggable network implementation.

Currently Untangled restricts which reconciler options can be sent for the Om reconciler. While I understand the desire to prevent users from doing bad things, his limitation made for a while be impossible to use shared, that now is added. I'm trying to develop a React Native app with Untangled, and then I got stuck into the reconciler options again, this time I need to send :root-render and :root-unmount to be able to render in React Native.

I propose we add a :reconciler-options to new-untangled-client, which should be a map that will be merged into the reconciler options, this way we fix the issue and any future issues regarding possible changes on reconciler for the future.

I have a repository where I could very easily reproduce the issue:

https://github.com/mitchelkuijpers/untangled-template-workspace/blob/master/src/client/template/ui/main.cljs#L42

Now started-callback called just after application creation, but union expansion done after mounting application.

As result it's impossible to use started-callback for additional initialization, e.g. initial routing.

Now that we're on the latest release, (:networking app) doesn't allow direct access to the xhr-io object, so we can't use the error handler we had set up before. This would be fine since network_error_callback is available now, but the network_error_callback gets back only the body of the error, not the status code (and status code might be useful in some circumstances). Either the status code should be passed into the network_error_callback, or we'll need some way to access the xhr-io object to get the status code ourselves to keep our error callback working.

So i'm having transient issues with load-field-action when it's used twice back-to-back
if I load-field-action on field :x, and load-field-action :y, in the app-state :x will loaded with value :untangled.client.impl.om-plumbing/not-found and :y will be loaded with the right value from the backend.

Other cases I tried:

1. load-field-action :x alone => loads correct value into :x from the backend
2. load-field-action :y alone => loads correct value into :y from the backend
3. load-field-action :y, then :x => ::om-plumbing/not-found for :y, loads correct value for :x,
4. load-field-action :x, then :y => ::om-plumbing/not-found for :x, loads correct value for :y
5. load-field-action :y, wait at the REPL for 1 second, then load-field-action :x => loads correct value for both :x and :y

here's my current backend code that is used to reproduce this bug (case 3 and 4)

(defmethod api-read :entitlements
  [{:keys [parser query] :as env} key params]
  {:value (parser env query)})

(defmethod api-read :ent-profile
  [env key {:keys [sso-id]}]
  {:value (auth/profile-by-id! sso-id)})

(defmethod api-read :ent-details
  [env key {:keys [sso-id]}]
  {:value (auth/entitlements-by-id! sso-id)})

(it's a recursive parser)

here's LOG output from the backend:

16-08-17 23:49:31 fabric INFO [admin.system:21] - Read Request: [ :entitlements [(:ent-details {:sso-id "54d2497fe3e8d00800893017"})]] 
16-08-17 23:49:31 fabric INFO [admin.system:21] - Recursive Read Request: [ :ent-details  ] ({:sso-id "54d2497fe3e8d00800893017"})
16-08-17 23:49:45 fabric INFO [admin.system:21] - Read Request: [ :entitlements [(:ent-profile {:sso-id "54d2497fe3e8d00800893017"})]] 
16-08-17 23:49:45 fabric INFO [admin.system:21] - Recursive Read Request: [ :ent-profile  ] ({:sso-id "54d2497fe3e8d00800893017"})

we get 2 read requests, in this case :ent-details came first, so it gets assigned :untangled.client.impl.om-plumbing/not-found in the app-state

client code:

(do
     (transact! [(entitlements/load-profile {:sso-id "12345"})])
     (transact! [(entitlements/load-entitlements-details {:sso-id "1234"})]))

this reproduces the bug, even though the load-field-actions are in separate transactions, if however at the REPL, I evaluate the first transact!, and wait a little bit, then evaluate the second transact! then the bug doesn't reproduce (both fields get loaded with data)

finally if I change my api-read code to the following (always return both fields, even though api-read is called once per field, and is suppose to return just the data for the field it was invoked for) , then the bug goes away

(defmethod api-read :entitlements
  [{:keys [parser query ast] :as env} key params]
  (let [sso-id (-> ast :children first :params :sso-id)]
    {:ent-profile (auth/profile-by-id! sso-id)
     :ent-details (auth/entitlements-by-id! sso-id)}))

When hooking into the global error callback, there are two parameters passed in, the app-state, and the error/status code returned from the server. It would be nice to have access to the most recent changes to the app state with regard to server errors. The modification would involve reordering the calls to the global-error-callback and the error-callback seen here:

https://github.com/untangled-web/untangled-client/blob/develop/src/untangled/client/impl/network.cljc#L63

the error-callback is the function that does the insertion of the :untangled/server-error which is implemented here:

https://github.com/untangled-web/untangled-client/blob/develop/src/untangled/client/impl/data_fetch.cljc#L409

Any issues with reordering these two calls?

I'm trying to debug an error we sometimes in our app:

Uncaught Error: No protocol method IWithMeta.-with-meta defined for type cljs.core/Keyword: :untangled.client.impl.om-plumbing/not-found

More information here might be helpful in debugging the message. Apparently likely culprits are client queries trying to access properties no longer available on the server, or a client trying to make a query with tempids.

When transact! with :params I see {:nil nil ....} in the app-state after the mutation finishes.
If I remove the named parameter :params then I no longer see :nil nilin the app-state

(transact! `[(~'untangled/load {:query [:permalink] :params {:permalink {}}})])

same story with load-data-action

(defmethod mutate 'permalink/create [{:keys [ast state] :as env} k p]
  {:remote (df/remote-load env)
   :action (fn []
             (df/load-data-action state '[:permalink]
                                  :params {:permalink {:app-state {}}}))})

I see :nil nil when :params is present.
Tested on 0.5.4-SNAPSHOT, (also it doesn't make difference whether :marker is true or false)

{:todos {:list/title "main",
         :list/items [],
         :list/filter :list.filter/completed}}

TodoItem query => [:db/id :item/label]
TodoList query => [:db/id :list/title :list/filter {:list/items TodoItem}]

(df/load :todos TodoList {:params {:list "main"}}

Results in :todos being wiped so that :list/filter does not exist after the load merge.

The merge-ident function we install in the reconciler does not honor our merge logic story. This was an oversight that was recently updated (on develop), but I don't think it is finished yet.

Specifically: If your query has :ui/attr (which are not sent to the server) it seems that the UI state value gets overwritten.

Merging by ident should merge just like the tree merge.

The built in untangled.client.mutations/set-integer! is pretty handy.

Wouldn't a set-bigdec!, set-bigint!, and set-instant! be useful also?

I can submit a PR for them if you'd like.

Edit: I don't think big decimals or big integers are supported. Perhaps you can think of a work around? Or set-double! at the very least?

If you query the server via:

[{[:id 3] [:x]}]

and the server returns nothing, then the mark stage makes this:

{ [:id 3] :u.c.i/not-found }

as the thing to merge, which crashes Om merge (the implication is to remove the item from the db table).

Untangled needs to define the main parser, but it would be possible to allow people to chain in parsers of their own in front of the built-in one.

I changed nothing between 0.5.7 and 0.5.8-SNAPSHOT (added it to have the devcards niceness with atoms and displaying them) And I noticed some of the views with unions and initial-state stopped working. I cannot share the complete code but this is basically what is happening in the app-state:

0.5.7

{:entity/current-tab [:ui.entities.current-view/overview 0],

0.5.8-SNAPSHOT

{:entity/current-tab {0 :ui.entities.current-view/overview},

Transmitting a support request is currently very expensive. We could compress history into a single snapshot followed by a sequence of diffs (one per tx).

The second kind of compression needed is being able to mark a transaction as "skippable", primarily for form-text input (e.g. every keystroke makes a new history entry). The rules would be: two or fewer adjacent "skippable" entries are left alone. Three or more cause all the "middle ones" to not be recorded at all such that you always end up with one or two "skippable" entries in history for any sequence. The first (and only) is the "one" case. The first and last are the "many" case.

Implementation-wise, this is essentially "overwrite the last history entry if that entry is skippable and the prior entry is ALSO skippable".

Might be nice to get this latter bit into Om as a PR somehow.

+the server must returns nothing for the ident to reproduce this

jasonjckn [3:27 PM]
it's weird to see anything at all in the global state at [:search :tab :hits [:search :tab]] for the ident [:search :tab], because if I actually returned a value from the server for [:search :tab] it would not exist in the global state at the path [:search :tab :hits ...] the data would be merged into [:search :tab], and then when om.next computes the props for the query [... [:search :tab]] it would fill it in

[3:27]
but as it stands I see [:search :tab] not found in the global state under hits

When using U.C. in a devcard, it would be nice if InitialAppState could be told to go INTO an externally supplied atom instead of ignoring the atom. This would allow the card to show the app state with :inspect-data!

Excluding the user namespace from refresh dirs makes dirwatch work right, but make-system then never changes and hot reload is nearly useless...it reloads code, but the old version is still linked. Adding user to the refresh dir causes reset to break.

When you try to do a server load from a mutation (specifying app/load in remote and load-data-action in action) it works properly, but the app/load gets sent over the network instead of being filtered.

If I have a component with the query
[:a :b :c]
and run a data-fetch/load-data against it with an :ident specified, the app state while the data load is going on looks like:
[:a ... :b ... :c ... nil <fetch marker>].

I believe this is a bug with the data-path function.

Conversation with @egracer
yeah. not sure if it’s a bug or not but it appears to be an issue with the data-path function that you linked me

[9:55]
it assumes that a load-field is happening, whereas your case is closer to to load-fields (plural)

[9:56]
so the field key may be empty but the query key may be present

[9:56]
makes me think that instead of having a special field key in the data state, that we should just put it under query

[9:57]
I’ll have to do some digging to see how / if to fix this but don’t have time at the moment. @gardnervickers can you file an issue and copy this little 5 line rant into it?

The current app-state merge function, defined in Untangled but passed to Om.next, could be optimized. For example, when we have a blob of data a shallow merge will do just fine, no need to scan for idents etc.

Ideas

• Compiled specter transform for tempid replacement.
• Shallow merge anything that is at a raw (non-join) prop (need the parser).

The function puts the items to load in a set, which will often cause them to reorder. This may cause post-mutations to run in an order that the user would not expect. This could cause dependent reads to be missing data on post mutation.

It works for unions one level deep, but does not work for nested ones

Need to change the macro to do something more like in the om next helpers:

bhauman/devcards@234a91d

The error state is placed in the app state, but no re-render happens so you cannot see it.

At the moment there is no way to add custom hooks to extend how transit works in Untangled Client. It would be fairly simple to add a parameter to the provided networking, though you'd have to create an instance of networking and pass it in.

Tracing back an error after bumping to 0.5.3, I come back to here: https://github.com/untangled-web/untangled-client/blob/master/src/untangled/client/core.cljs#L227

Seems like it should be fine, but when turning on advanced compilation, it stops working. After a bit of time investigating how protocols and om.next's static version of them works, I believe that it is not valid to call om/ident directly if you want advanced compilations to work - see how om/get-query works for om/IQuery ( a very similar case to the om/Ident protocol):

https://github.com/omcljs/om/blob/747655fe65b4a46a502d9b76171642ef0f67d532/src/main/om/next.cljs#L302-L309

If someone executes the same remote mutation more than once per processing loop:

• The mutations appear in one tx on the wire
• The server has to return a map keyed by mutation name
• We lose all but one return

Given this code

(df/load-data reconciler [:env-vars :login-profile :entitlements] :marker false)

I setup a diff algorithm on my app-state atom called in a watch, that prints to console the deltas of @App-State over time. The output of this is attached in console screenshots, you can see :env-vars (but not :login-profile or :entitlements) has a marker put in before loading the data. You can see the marker put in the very first entry in that pic (the marker is inside {...}). The second pic shows the contents of the marker. (NOTE: it's interesting that there's no markers for the other keywords in df/load-data, just :env-vars).

Sometimes I have to get some information from a remote mutation that I would like to handle in a post-mutation. Most of the time can be solved with optimistic updates. but we have some use cases where that won't work.

Currently I see no way of accessing the mutation response, so I think this could be a good enhancement.

We're getting an error:

Uncaught Error: No protocol method IWithMeta.-with-meta defined for type cljs.core/Keyword: :untangled.client.impl.om-plumbing/not-found

When we try to use data-fetch/load-field-action or associated functions on an item with a tempid in the ident, we end up getting an error, even though the item's idents are correctly rewritten in most of the request queue. This turns out to be because data-fetch creates a closure around a list of items that are being lazy loaded, and closes around these tempids in a way that clojure.walk can't find and replace.

Throwing an exception from a read/mutate is supposed to work as a method of triggering error handling on client, but the error path of the server seems to fail to encode the response in transit (it bypasses it), and the client crashes as a result.

I saw a strange case where a not-found marker ended up in app state. May have messed up the mark/sweep when adding progressive load.

right now the ordering of Map entries on a TabUnion component can actually influence the initialized global atom

These two query expressions result in different initial global atom states

(query [T] {:listings (get-query ListingsTab)
                 :bookings (get-query BookingsTab)
                 :messages (get-query MessagesTab)
                 })

(query [T] {:listings (get-query ListingsTab)
                 :messages (get-query MessagesTab)
                 :bookings (get-query BookingsTab)
                 })

Both MessagesTab and BookingsTab have a join in their query expressions {:results (get-query SearchResults)} and my SearchResults is a singleton with ident [:search-results :global]
So depending on which Tab initial-state is merged last determines what the normalized data inside SearchResults will be in the global atom. The ordering of the Map entries is influencing the ordering of merge-state! calls which is influencing the global atom final initialized state since both BookingsTab and MessagesTab have state that gets merged into [:search-results :global]

My suggested solution is to guarentee that initial-state of TabUnion is the last one to be merged in. All the other tabs have their initial state merged first.

The new auto-initialization of unions in 0.5.3-SNAPSHOT is not properly normalizing.

The lazily-loaded function was supplied as a way to show custom loading renderings in place of a component when network loads are in progress. Unfortunately, we coded it to swap itself in/out for the component itself, which is a no-no in Om land (since you lose the real component while the loading marker replaces the component in the DOM, leading to an Om error).

Need to change this function to be used from within the given component, and fix the design. Unfortunately, the to-many case (loading a list) may cause complications....definitely requires more thought.

https://gist.github.com/gardnervickers/687da840630c07d1e4f9721201a051c3
Here's the devcard app state

The gist contains a repro, tested on 0.6.0 and 0.6.0-SNAPSHOT. Also happens with the cookbook tabbed union example.