Tempid resolution for tempids that have been queued may be broken

[awkay]

https://github.com/fulcrologic/fulcro/blob/develop/src/main/com/fulcrologic/fulcro/algorithms/tempid.cljc#L114

This may be the wrong ns. I've had reports that a tempid inside of a txn is not properly resolved.

[awkay]

There was a typo in tempid remapping code that is supposed to fix tempids for non-application state (queues, etc.). Should be fixed in 3.5.19-SNAPSHOT and above. Needs testing.

[TimovanderKamp]

Breaks tempid->realid map in queues

; Server side resolver

(pco/defmutation something [env {:keys [id]}]
  {::pco/op-name 'dosomething}
  {:tempids {id "Resolved ID"}})


; Client side code

(defmutation dosomething [_]
   (remote [env] 
        (m/with-server-side-mutation env 'dosomething))
   (ok-action [{:keys [tempid->realid]}]
        (println :tempid->realid tempid->realid)))


(dom/div {:onClick (fn []
                       (let [temp (tempid/tempid)]
                         (comp/transact! this [(dosomething {:id temp})])          ; (mutation 1)
                         (comp/transact! this [(dosomething {:id temp})]))})      ; (mutation 2)

Expected print result from ok-actions:

1. {(tempid/tempid) "Resolved ID"}
2. {(tempid/tempid) "Resolved ID"}

Actual print result from ok-actions:

1. {(tempid/tempid) "Resolved ID"}
2. {"Resolved ID" "Resolved ID"}

[awkay]

Ah. Hm. Yeah, the first txn remaps that tempid everywhere, including in the queues. Going to need an exclusion for that.

[awkay]

But what, exactly, got sent on the network by (2)? I think it doesn't matter, because the second one should have communicated with the server with the correct ID, which means there is no longer a tempid possible in anything.

[TimovanderKamp]

I think that the main problem is that the mutation's parameter is a tempid and that it is not possible to resolve the realid if you want to perform an ok-action.

(defmution something [{:keys [id]}] ; tempid here
  (ok-action [{:keys [tempid->realid]}]
    ; not possible to know realid to trigger a load for example
))

You're correct that essentially there is no tempid resolution in the second remote mutation.

However, it would be ideal to have prior tempid resolutions available in queued mutations.

[awkay]

Technically you are correct, but there are ways to work around it. For example, using returning from a mutation is really the right way to "load" the result of a mutation.

If you wanted to take a stab at a fix, it's probably just changing the prewalk-replace in resolve-tempids in algorithms.tempid. Basically you still need to find the tempids everywhere, but skip values where the key is :tempid->realid. A bit fiddly given that that is supposed to work on general data structures, so the tempid could appear in a set, vector, map key, map value, or even list.

So, it would have to be rewritten very carefully with decent tests.

I'm not absolutely sure that's the internal value that you'd need to track.

[awkay]

By the way, have you looked at the id in the second mutation's params? Technically it should have already been rewritten as well. The closure over that value when into the tx queue, and should have been rewritten...so I would not expect you to need to rewrite it.

[awkay]

OK, I've verified what you're seeing, and I can confirm that it does not work in an ideal manner. There is no easy fix internally for this, but there is an easy workaround: put the follow-on transact call in the first mutation's ok-action.

For future reference, here is the issue (in case I get the energy to spend a couple of days reworking the internals):

• A mutation is just a multimethod that returns a map.
• That multimethod closes over the params, so they cannot be rewritten by the tempid system here:

https://github.com/fulcrologic/fulcro/blob/79eb98387fd72c8924f2a58267c43c0a9af72f4a/src/main/com/fulcrologic/fulcro/mutations.cljc#L447

• The tempid in the submitting second transaction is not closed over, so the remapped value gets returned to the server (correctly) but the param still has the tempid value. Thus, an ok or error action doesn't know the ID of the entity that was actually affected in app state. It could, if there's only one tempid, infer it from the tempids map, since both k and v and the actual real id....but this is an inconsistency that is annoying.

[awkay]

After messing with it for some time I've decided there is no clean solution for making this work. The official fix is to submit the second transaction from the ok-action of the first. I'm not willing to rewrite the tx system for a minor inconvenience that I've never hit in 5 years of production use.

Remember that:

• Mutations can use returning to get back data about the thing on the server
• Tempids are resolved in a distributed async fashion, so IMO if you have a combination of things you need to do to some new thing, then it should be part of a single mutation on the server, where the overall interaction can be managed. What if the second one failed due to network outage? Then you'd have a partially correct result on the server.