Relay: feature: Change the retain function in RelayModernStore

[morrys]

I wanted to propose to modify the retain function in RelayModernStore so that it can receive the following information:

• the CacheConfig parameter of the query
• a boolean that indicates if the method is called by lookupInStore or during the execution of the query in the network

as is

retain (selector: NormalizationSelector): Disposable

to be

retain (selector: NormalizationSelector, retainConfig: {network: boolean, cacheConfig ?: CacheConfig}): Disposable

This change allowed me to extend the store and implement the TTL management of the queries (we can expect to open an issue to bring this management natively in relay).

Thanks,
Lorenzo

[josephsavona]

Hmm, we're actively exploring new features to allow finer grained control of cache invalidation, and also looking at changes to the Store API to support a new approach to loading connections. We considered query-level TTL but aren't convinced that's the right long-term API, so I think we need to understand the use-case better - probably the first step is documenting our thinking around cache invalidation so that you and others can give feedback.

[morrys]

Hi @josephsavona, I have noticed many changes in the management of the Store and I would like to discuss some of its aspects before all the new experimental features become official.

• A change that I really appreciate is the new management of roots in the retain function:
  It is no longer managed through an index but using the new property which identifies the request, very similar to the implementation in the store I use to manage the persistence of the Relay store.

• TTL vs gcReleaseBufferSize
  This theme is certainly the most complex to deal with and for the moment extending the retain function is the simplest solution.

• A change that would force me to have to fork the Store is the evolution of the object saved in the roots and its strong use in the Store.
  I'd like to propose you to modify the roots object from Map in MutableRecordSource and provide a new option in the constructor to allow us to use a custom implementation.

• Same goes for the new object _connectionEvents

What I ask is to make all the objects that guarantee the consistency of the Store of objects of type MutableRecordSource and configurable. (I am available to perform the PR)

[josephsavona]

Hi @morrys, it sounds like the main theme of your request is that you want to be able to override (and access) all of the Store's data, not just the RecordSource object that holds the cached records. Note that we intentionally _don't_ use RecordSource for things like roots or connectionEvents - those don't store Records so they require a different data type like Map - but in theory we could allow the user to provide the values.

Overall, though, I think writing your own Store is the best option. We intentionally defined a Store interface separately from the default implementation and allow the user to pass their store instance when creating an environment, precisely to enable experimentation with store implementations.

Note that we have some upcoming APIs around data-invalidation that @jstejada is building out, which we think can sometimes be more flexible than per-query TTL, especially when combined with the release buffer size. I'd encourage you to follow along on those commits!

[josephsavona]

I'm going to close as we won't proceed with this change as-is, but feel free to continue the discussion here - we're definitely excited to see your and others work on supporting the use of Relay when offline or in poor network conditions!

[stramel]

Note that we have some upcoming APIs around data-invalidation that @jstejada is building out, which we think can sometimes be more flexible than per-query TTL, especially when combined with the release buffer size. I'd encourage you to follow along on those commits!

@josephsavona @jstejada Did the changes you alluded to land?

[sibelius]

You can use data invalidation since v8 (https://github.com/facebook/relay/releases/tag/v8.0.0)

https://relay.dev/docs/en/relay-store#invalidatestore-void

https://relay.dev/docs/en/relay-store#invalidaterecord-void

[morrys]

@sibelius the invalidation of the store is very different from the TTL, the invalidation allows you to define what should be considered stale while the TTL define what should be considered available.

[morrys]

@stramel it seems that TTL will be introduced in relay-runtime
https://github.com/facebook/relay/commit/bb888069cd238b10516bd1ddc528b8f84bdf408f

[jstejada]

yes, we added a global per-environment query ttl value, which will determine how much time need to pass from the moment a query was written to the store before it is considered stale (similar to as if it had been invalidated with data invalidation)