Closed mstenta closed 4 years ago
This is weird. I've looked around and experimented with things and am getting wildly erratic results; sometimes reproducing this error, sometimes not.
I've found some similar issues reported with Vue's v-model="foo"
directive, which recommend using :value="foo" @input="foo = $event.target.value"
instead (v-model
is essentially just syntactic sugar for that), but we're already using the syntax for all our forms, since we're using Vuex for log data instead of component state. Another caveat from Vue's docs on Form Input Bindings:
For languages that require an IME (Chinese, Japanese, Korean, etc.), you’ll notice that v-model doesn’t get updated during IME composition. If you want to cater to these updates as well, use the input event instead.
But again, this amounts to essentially the same advice as above. Soooo... :man_shrugging:
Afraid to say I'm gonna punt on this one though until after 1.0.0 or later.
Hmm now i'm seeing the same behavior in my browser on my laptop as I'm typing on my regular keyboard. :-(
So this might not just be an Android glide typing issue.
Bumping this up as a blocker to v1.0.0 because it makes the text fields almost unusable. I'm curious if I'm the only one experiencing this. It's happening consistently in Firefox on Android and Ubuntu.
Hm, I'm still having trouble reproducing this consistently. I can't even reproduce it with glide typing now. I've tried it on Android Chrome (with multiple keyboards), Ubuntu Chrome and Ubuntu Firefox. No issues.
Can you describe the behavior as you're experiencing it on your laptop with a physical keyboard?
Have you looked to see if there are any errors appearing in the console on Ubuntu Firefox?
Ok, after a call with @mstenta, I think we've identified some likely causes.
Initially, I thought the main culprit was farmLog.updateLog
, because it's a computationally heavy operation. Thinking about this more, that may certainly be exacerbating the problem (and could be why this problem has only surfaced now), but I think the heart of the problem is too many asynchronous function calls being used to propagate changes through the Vuex store, with no means of retaining the original sequence of the input events.
It seems very likely to me that there are race conditions between the initial input event, the updateLog
Vuex action, the updateCachedLog
action and the final DOM rendering. The input event and updateCachedLog
action are very obviously asynchronous, but critically, so is updateLog
, since all Vuex actions are asynchronous (ie, they always return a promise).
I'm oversimplifying, but we essentially have 4 steps for that must occur each time a log updates:
1) The EditLog
component's updateCurrentLog
method dispatches the updateLog
action with the log's localID and its new properties.
2) In the Vuex store, the updateLog
action asynchronously looks up the log in the store by its localID, then with that reference to the log's current state and the new properties, makes an updated copy using farmLog.updateLog
, setting both isCachedLocally
and wasPushedToServer
to false
. It then commits the the copy to the store, using the addLogs
mutation.
3) Whenever addLogs
is committed on a log with isCachedLocally
set to false
, that triggers a subscriber that calls updateCachedLog
4) updateCachedLog
makes an asynchronous call to the db, and whenever that finishes, it will commit addLogs
again, this time with the same updated log but with its isCachedLocally
property to true
.
So let's say we have 2 successive updates from the input field, first Update A then Update B, and they're updating the name
field, which prior to Update A was just "t"
; Update A gets an input value of "th"
; Update B gets an input value of "the"
. The sequence of events is intended to look like this:
Update | Step | log in Vuex |
log in IndexedDB |
---|---|---|---|
A | 1 | { name: "t", isCachedLocally: true } |
{ name: "t" } |
A | 2 | { name: "t", isCachedLocally: false } |
{ name: "t" } |
A | 3 | { name: "th", isCachedLocally: false } |
{ name: "t" } |
A | 4 | { name: "th", isCachedLocally: true } |
{ name: "th" } |
B | 1 | { name: "th", isCachedLocally: true } |
{ name: "th" } |
B | 2 | { name: "th", isCachedLocally: false } |
{ name: "th" } |
B | 3 | { name: "the", isCachedLocally: false } |
{ name: "th" } |
B | 4 | { name: "the", isCachedLocally: true } |
{ name: "the" } |
However, it may actually be executing in this sequence:
Update | Step | log in Vuex |
log in IndexedDB |
---|---|---|---|
A | 1 | { name: "t", isCachedLocally: true } |
{ name: "t" } |
A | 2 | { name: "t", isCachedLocally: false } |
{ name: "t" } |
B | 1 | { name: "t", isCachedLocally: true } |
{ name: "t" } |
B | 2 | { name: "t", isCachedLocally: false } |
{ name: "t" } |
A | 3 | { name: "th", isCachedLocally: false } |
{ name: "t" } |
A | 4 | { name: "th", isCachedLocally: true } |
{ name: "th" } |
B | 3 | { name: "te", isCachedLocally: false } |
{ name: "th" } |
B | 4 | { name: "te", isCachedLocally: true } |
{ name: "te" } |
There are a few places where asynchonisity could be biting us in the butt; step 4 is the most conspicuous, but I think step 2 might be even more consequential, because once the updateLog
action is called it essentially forks execution down separate, asynchronous paths. So if the second input event is fired off quickly enough, it will reach step 2 before the first update has reached step 3, which is why we end up with Update B producing the state "te"
instead of the expected "the"
.
One thing @mstenta and I noticed was that this bug was easiest to reproduce if you refreshed the "Edit Log" screen and immediately started typing in the name
field, particularly while network requests were still running in the background. My first thought was that there was some sort of memory issue, and that the repeated execution of farmLog.updateLog
in combination with those network requests were overburdening memory. I realize now, however, that the behavior had much more to do with numerous asynchronous calls flooding the event loop all at once, causing even greater latency between input events and the eventual DOM rendering, which ultimately enabled the race conditions. The same behavior can be observed if you just type extremely fast.
I'm still not 100% sure the best way to remedy this, but I think I've got a better handle on why this is happening now.
Some next steps towards resolving this:
logTypes
from shellModule
to farmModule
, so it's part of the state
available from the addLogs
mutation.updateLog
action into the addLogs
mutations, so it's all synchronous.isCachedLocally
metadata from logs (see #385).farmLog.updateLog
a little faster by only iterating over the keys provided in props
.updateLog
. #358 Move
logTypes
fromshellModule
tofarmModule
, so it's part of thestate
available from theaddLogs
mutation.
In the course of this I'm totally overhauling how the logTypes
are updated from the server and cached locally, separating it out from the updateSiteAndUserInfo
and loadCachedUserAndSiteInfo
actions. For one thing, I'm storing everything as resources
, instead of just stripping out the logTypes
and discarding other information for assets, areas, etc. I also want to implement some smarter checks on exactly how those resources
get updated, but I'm torn how to do so. For the time being, I'm just using Ramda's mergeDeepRight
to merge the old resources
with the new, but I'm wondering if this could be prone to errors. I'm trying to think what is the best way to handle changes to the server configuration of log types so that those changes will be reflected in FK, while also making sure we have a way to migrate logs that might be effected by those changes. I'm struggling to understand how and why those changes might occur, so it might be good to have a talk with @mstenta to figure out how best to accommodate such changes.
I use Android's "glide type" feature for typing into text fields on my phone (spell out a word by dragging my finger over all the letters). I'm finding that this is buggy in Field Kit. After "glide typing" a word, it sort of flashes and then disappears, and I have to do it again. It is pretty consistent, but sometimes the word does stick.
I have to assume this is something to do with the way Vue.js is monitoring/propagating changes to the input fields, but I don't know enough to be able to test further. I tried testing the same glide typing in this example Vue.js form and did NOT experience the same issue: https://vuejsexamples.com/simple-vue-js-contact-form-2/