https://kotlinlang.org logo
Join SlackCommunities
Powered by
I think from my original list for kotlinconf, I on...
# confetti
y
I think from my original list for kotlinconf, I only have to update the tile and complication. I'd like to land my PRs and build on top of those APIs. There is one contentious change with https://github.com/joreilly/Confetti/pull/556, but it's only used on Wear. Any objections to me landing and following up on feedback. Mainly some concerns from @mbonnin
Heads up @John O'Reilly, about to land those PRs without major review, and apologies if I need to revert, but want to get this completed.
m
Go for it, I'll take a look this afternoon
j
Just kicking off publication to play store (internal test) with those changes
should be there now
@yschimke I just published another update that includes https://github.com/joreilly/Confetti/pull/561
m
Quick question: did we decide to use 
FetchPolicy.CacheAndNetwork
by default (based on the PR above, I think it is now the default parameter)? I'm asking because that was raising a crash during my tests (not sure why, but @mbonnin knows it).
m
I'm pretty much against 
CacheAndNetwork
because: • it doesn't work with 
execute()
as it can produce 2 responses • it changes the UI without user action
If we want to use it why not, but I'm not sure I see any upside to using it
m
I'm more concerned with the "produce 2 responses" as that scenario was crashing the app for me with an exception:
Copy code
The operation returned multiple items, use .toFlow() instead of .execute()
m
I think @yschimke changed most of the 
.execute()
to 
toFlow()
and uses flow operators to hide errors upstream (see ClientQuery.kt)
I'm still processing all of that but my initial hunch is that everything would be much more easier if we 'just' got rid of 
CacheAndNetwork
There are 3 cases: 1. One-item: 
client.query(query).fetchPolicy(CacheFirst).execute()
2. Two-items: 
client.query(query).fetchPolicy(CacheAndNetwork).toFlow()
3. Ongoing flow (never terminates): 
client.query(query).watch()
Tangential question: what do we want to use the 
Repository
for ? Right now it's just a class that centralizes all calls to 
ApolloClient
but doesn't have a lot of logic
y
Right now it's just a class that centralizes all calls to 
ApolloClient
but doesn't have a lot of logic
I'm ok with this, or removing it because everyone just submits GraphQL queries. I want to avoid a separate layer, and hope we can mostly push down the complexity onto Apollo. Screens just define the queries the need to get data.
I'm pretty much against 
CacheAndNetwork
because:
• it doesn't work with 
execute()
as it can produce 2 responses
• it changes the UI without user action
I'm against it for other reasons. The two responses are fine, because you don't know if the network is going to work, so if you want fresh data (which the mobile originally did) then it's good to show immediate results from the cache, but then refresh. I think dealing with a Flow in your UIs is common, and it's way harder to start with suspend forms like execute(). But I'm against it, because I think this app is a great candidate for an offline first app, relying on Apollo to do the right thing.
m
I think dealing with a Flow in your UIs is common, and it's way harder to start with suspend forms like execute().
yea, fair point
Screens just define the queries the need to get data.
That works for queries that ultimately terminate. Not for 
watch()
y
If the repository is just accessors for well defined ApolloCall functions, then the issue only arises in the ViewModel. A view model that wants to call execute, watch or toFlow, can because it also specified the fetchMode
That works for queries that ultimately terminate. Not for 
watch()
If I read watch() it seems like it doesn't hit the network, just keeps in sync with the local cache, is that correct?
m
Yep, correct
(mostly)
There's an initial call that can hit the network
Because we need cache keys to listen to so we need initial data
Otherwise it's listening to each and every cache change and trying to run the query every time. Which kind of works but not optimal
then it's good to show immediate results from the cache, but then refresh.
I have mixed feeling about this. It's okay if the data doesn't change too much but the twitter example is a good example of weird behaviour when you're reading a tweet that suddendly disappears. This is infuriating to me
If the refresh doesn't change the UI it's fine though
y
OK, but maybe the toUiState function is correct, if screens choose to use CacheAndNetwork (and deal with updates intelligently). so perhaps we just agree to use CacheFirst on all screens in the app to avoid that glitchyness?
m
Yep, CacheFirst is good 👍
I think with background refresh using WorkManager + pull to refresh it gives good control over data freshness
There's one consideration that not all the screens currently implement pull to refresh but I think in a perfect world that be nice
j
Do you see many GraphQL deployments that use subscriptions?
I guess that would typically only be while app is in foreground....
(thinking in terms of different approaches to keeping data fresh)
m
Not too much TBH, it's expensive to run on the server
The use cases are chat apps or other things that really require realtime
Or maybe if you want to keep a like counter fresh without polling
Stuff that changes in the ~second scale
Our data is more ~minutes
j
Ideally I guess it would be nice to see something like a bookmark change sync across your devices
(without needing polling)
m
Yea maybe for stuff like this
Collaboration on bookmarks 🙂
Will require handling conflicts too
j
true
it's an interesting area and probably accounts for good part of complexity of many mobile apps
y
Back to the topic in the original post. This updates Tiles and Complications to use Bookmarks. https://github.com/joreilly/Confetti/pull/566
But I'm not happy with where it finished. Better than existing functionality, but not compelling.
I'd like complications to be showing avatars, but for now, just have boring text.
3 Views
Open in Slack
PreviousNext
Powered by