Support Date instances in replaceEqualDeep function

[olederle]

Our API layer is replacing string dates with Date instances already. However, now we loose the whole structural sharing capability of react-query, because: Structural sharing only works with JSON-compatible values. So as soon as we have a date in a resource, it is always considered changed, even if it is absolutely identical. This becomes particularly problematic in collections or nested structures. That is also the reason the function isDataEqual as proposed in discussion #3559 is not a solution we can use. I know this function, but it allows me only to declare a whole collection as equal or not. If there is just one entry with a different date, the function must return false. But then the default replaceEqualDeep function is used, where I do not have any possibility to get involved, and so I get a collection with all entries changed.

The problem is, that Javascript supports serialization of Date but can't provide the same for deserialization. But we do not want to have this inconsistency and suddenly being confronted with string values that actually should be dates. That's why our deserialization supports Date as well, and we do not want to loose this.

At the moment we have to do some patching of replaceEqualDeep with something like this at the very end of the function:
if (a instanceof Date && b instanceof Date && a.valueOf() === b.valueOf()) { return a; }

This would not be necessary if either:

• the replaceEqualDeep would support the Date type by default as well
• a custom replaceEqualDeep could be provided the same way as it is possible with isDataEqual (if you don't want the date type in the default implementation)

Even if I would move the date transformation above the react-query layer, that would not work unless i use a second cache layer. If I change the instances returned by react-query, the replaceEqualDeep would not work again the next time. So I have to create a second copy of the data. And that is also not a good solution in my opinion, because react-query does this very well already.

query/src/core/utils.ts

Lines 346 to 373 in 9f6c992

	export function replaceEqualDeep(a: any, b: any): any {
	if (a === b) {
	return a
	}
	const array = Array.isArray(a) && Array.isArray(b)
	if (array || (isPlainObject(a) && isPlainObject(b))) {
	const aSize = array ? a.length : Object.keys(a).length
	const bItems = array ? b : Object.keys(b)
	const bSize = bItems.length
	const copy: any = array ? [] : {}
	let equalItems = 0
	for (let i = 0; i < bSize; i++) {
	const key = array ? i : bItems[i]
	copy[key] = replaceEqualDeep(a[key], b[key])
	if (copy[key] === a[key]) {
	equalItems++
	}
	}
	return aSize === bSize && equalItems === aSize ? a : copy
	}
	return b
	}

[TkDodo]

once we start supporting dates, we will also have to support Map, and Set, and basically everything that superJson supports. I really don't think I want to go down that road. It would be better to just put json into the cache, and apply all transformations after reading from the cache, e.g. with a custom hook.

[olederle]

I understand that you do not want to extend the default logic. But adding an extension point to provide a custom replaceEqualDeep implementation is not an option as well?
Doing the conversion above the cache layer seems not to be a good architectural choice and is not possible in our case, as the API layer provides us the converted data already. It's not possible to change this. And even if we would do this, implementing a whole second cache and reimplementing the structural sharing what react-query does already, is nothing I want to do neither.
So it seems that I have to stick with the current solution and patching the library. Something i would like to have prevented, but is the least effort to achieve what we need.

[TkDodo]

yeah, let's think about an option that allows that. I'm just thinking if that wouldn't make the isDataEqual option unnecessary, and / or the structuralSharing one as well. All those options kinda do the same? Maybe we should just have structuralSharing accept a function as well ?

[olederle]

In our specific case, the requirements are such that isDataEqual would be too strict and therefore we cannot use it. I don't know if there are other use cases where this could make sense. But I personally can't think of any. I think if there is a way to provide a separate function for structuralSharing, isDateEqual is no longer necessary. The only reason I can think of is to remain compatible in this respect. I don't know how much this option is used so far.

Regarding structuralSharing, I could imagine that there are 2 possibilities. Either one simply "activates" it and a standard implementation is used, basically the one that already exists. Or one defines an own function, which would also correspond to activating it.

[TkDodo]

Regarding structuralSharing, I could imagine that there are 2 possibilities. Either one simply "activates" it and a standard implementation is used, basically the one that already exists. Or one defines an own function, which would also correspond to activating it.

could we do this as a non-breaking change where structuralSharing can either be a boolean or a function?

[richsilv]

fyi, I'm in the process of implementing this.