queryStrict strategy is not very intuitive

[veej]

A query can use two different default strategies to match the cached values: a shallow equality (queryShallow), which compares every first-level field in an object (if the type of the input param is "object"), and a strict equality (queryStrict), which compares the inputs object with just a reference equality (===). However, the strict equality has a very limited usage, since it makes sense only if the query has a single flat parameter (i.e. not wrapped in an object), for example:

const query = queryStrict((foo: string) => apiCall(foo), expires(10000));

If we need to define a query with multiple parameters (that avenger forces us to wrap in a single object), or if we want to pass a single parameter wrapped in an object, it's practically impossible to keep the same reference for the input object, causing a lot of unwanted refetches even if we don't change the references for the single parameters.

const query = queryStrict(({ foo: string, bar: number }) => apiCall(foo, bar), expires(10000))

const foo = "test";
const bar = 20;

<WithQueries
   queries={{ query }}
   params={{ query: { foo, bar } }}    --> since we define a new object every time, 
                                           the input object will never match the cached entry
/>

Moreover, it's not very clear what happens when a query has no parameters. AFAICT, when calling a query without params, we match for an empty input object, that will always have a reference different from the one stored in the cache, so the query will always be refetched in case we use queryStrict.

To be confirmed: it also seems that the observables use always a shallow equality. This means that if two components calls a queryStrict with the same params (but wrapped in an object, so the reference will be different), the cache will not be matched when the last component runs the query, but the refetch will affect both components, since they're observing the same (according to shallow eq) input parameters. If the second component is a child of the first one, the re-rendering of the first component (due to the change of the observed query) may re-render the second, that will re-run the query (not matching the cache), causing again a re-render of the first component and so on, causing an infinite loop of re-renders.

[giogonzo]

Let's discuss this better offline probably, but as a TL;DR of what I got and what actions we could try:

• queryStrict should only be used for primitive input values - maybe we can enforce this at type level?
• for other scenarios, queryShallow can help if it's still a simple one-level nesting of primitive values, otherwise the correct way to go is to use the simple query and pass an explicit Eq

---

Commenting the post:

A query can use two different default strategies to match the cached values: a shallow equality (queryShallow), which compares every first-level field in an object (if the type of the input param is "object"), and a strict equality (queryStrict), which compares the inputs object with just a reference equality (===).

Minor clarification: I would not call these strategies but rather helpers built on top of the plain query, accepting Strategies.

However, the strict equality has a very limited usage, since it makes sense only if the query has a single flat parameter (i.e. not wrapped in an object), for example:

Indeed, this is strict equality in JS

If we need to define a query with multiple parameters (that avenger forces us to wrap in a single object), or if we want to pass a single parameter wrapped in an object, it's practically impossible to keep the same reference for the input object, causing a lot of unwanted refetches even if we don't change the references for the single parameters.

Indeed, this is the same problem you have e.g. with any hook accepting an array dependency - you will need to obtain some sort of memoized / stable reference to the object if you want to use a non-primitive as dependency. But yeah, the queryStrict helper is not meant for objects / non-primitive values. This could probably be added to the signature and enforced at compile-time by queryStrict? Also in this example (not sure if it's true for the real one), swapping in queryShallow would already mask the problem as we have a single level of nesting of primitive values inside the input object.

Moreover, it's not very clear what happens when a query has no parameters. AFAICT, when calling a query without params, we match for an empty input object, that will always have a reference different from the one stored in the cache, so the query will always be refetched in case we use queryStrict.

I didn't look for the exact answer to this question in code, but I would not expect this to happen, i.e. no one should touch the inputs for a specific query before checking the cache. Did you look into the code or is this comment based on observed behavior?

The last "to be confirmed" point is for sure worth investigating better, but:

observables use always a shallow equality

this shouldn't be the case, since an input should always be compared using its strategy's inputEq throughout the codebase.

If you are referring to this lines in DSL.ts:

import { observeShallow } from './observe';
export const observe = observeShallow;

please consider here "shallow" is referred to how subsequent outputs are considered equal / aggregated, not to comparing inputs.

[veej]

Minor clarification: I would not call these strategies but rather helpers built on top of the plain query, accepting Strategies.

Gotcha. I called them default strategies meaning exactly this, but it's clearer if we call them helpers.

Indeed, this is the same problem you have e.g. with any hook accepting an array dependency - you will need to obtain some sort of memoized / stable reference to the object if you want to use a non-primitive as dependency.

Hooks are indeed a good similarity: when a hook accepts an array of dependencies, we must keep stable references for the single elements in the array. When a query receives a list of params (represented as an object in avenger), we must keep a stable reference to the entire list. It's more like if React requires to keep a stable reference for the entire dependency array.

I didn't look for the exact answer to this question in code, but I would not expect this to happen, i.e. no one should touch the inputs for a specific query before checking the cache. Did you look into the code or is this comment based on observed behavior?

It's based on observed behaviour, but I could be wrong. Maybe the refetching was caused by other factors. I'll try to replicate the issue in vitro.

The last "to be confirmed" point is for sure worth investigating better, but:

observables use always a shallow equality

this shouldn't be the case, since an input should always be compared using its strategy's inputEq throughout the codebase.

If you are referring to this lines in DSL.ts:

import { observeShallow } from './observe';
export const observe = observeShallow;

please consider here "shallow" is referred to how subsequent outputs are considered equal / aggregated, not to comparing inputs.

Yes, that was indeed the line of code that fooled me. However, that was the only explanation we found for what we observed: a parent component polling a query, a child component (a modal in particular) fetching the same query (with the same inputs). The query was defined with queryStrict and an input object with just one field (the actual parameter). I would expect the child component to refetch the query only once (due to queryStrict + input object), instead the dialog started refetching in an infinite loop when opened, like if the query itself was causing a re-rendering of the component fetching the query.

[veej]

I did some tests around queryStrict + empty input list and it doesn't seem the query is refetched, so at least that part of my original report seems to be wrong (i.e. should be safe to use queryStrict when the query doesn't receive any input param.. probably the refetch in our project was caused by other chained events).

The issue with the infinite loop instead is still valid, and I will try to reproduce it in vitro with a bare minimum example.

[giogonzo]

We re-discussed this offline and came to the following conclusions:

• as an alternative to my proposal above "This could probably be added to the signature and enforced at compile-time by queryStrict?", we could also remove queryStrict altogether, since queryShallow already optimizes the basic === scenario with an ad-hoc check
• it still remains to debug better the loop spotted by @veej , even if it was only visible with a non-correct setup (queryStrict used on non-primitive inputs), it might still hide some actual bug underneath