Check if dates are equal in structural sharing

[juanrgon]

Closes: #6482

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
query	⬜️ Ignored (Inspect)	Visit Preview		Dec 3, 2023 4:46pm

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 8743ce8:

Sandbox	Source
@tanstack/query-example-angular-basic	Configuration
@tanstack/query-example-react-basic-typescript	Configuration
@tanstack/query-example-solid-basic-typescript	Configuration
@tanstack/query-example-svelte-basic	Configuration
@tanstack/query-example-vue-basic	Configuration

[TkDodo]

We only support json serializable values on purpose. If we start to support date, we also need Set / Map and everything else. You can always pass your own function to structuralSharing

[juanrgon]

We only support json serializable values on purpose. If we start to support date, we also need Set / Map and everything else.

@TkDodo I understand this rationale, but:

• I assume that Set / Map are less common than Date objects. I have yet to use a Set / Map in any of my API responses for my app (https://meetmotif.com)
• If this is a blocker, I can update this function to support Set/Map

You can always pass your own function to structuralSharing

I'm currently doing this in my project. However, it took me over 6 months to understand this was happening. I had the false assumption that react-query by design always triggered a render when any data was loaded, because virtually all of my queries include a date object (e.g. createdAt or modifiedAt)

I don't want to push too hard because there is a work-around, but I'm assuming that I'm not the only one who has this issue (regardless of if others know this or not)

Also, thank you for the quick reply! 🙇🏽

[TkDodo]

are less common than Date objects

Not really. The main use-case for TanStack Query is still to manage async state that comes from data fetching, where only JSON is supported. You're using superjson with trpc, and that's a different story. If we would need to support everything that superjson supports, we would need to probably add superjson itself, which means another 3kb of bundle size, which is not acceptable for a somewhat niche use-case.

My suggestion here is that this is something that trpc would need to handle if you don't want to handle this in user land. They create the QueryClient for you, so if they know that you use the superjson transform, they should also provide a better structuralSharing function.

For "vanilla" react query, this is definitely out of scope, sorry.

[tran-simon]

@TkDodo

You can always pass your own function to structuralSharing

Is there a better way to customize the replaceEqualDeep function? Currently I had to copy the whole function as well as the isPlainArray, isPlainObject and hasObjectPrototype (these are not exported by @tanstack/query-core) just to add the 3 lines to compare objects. It would be nice to have a factory function or something similar to create a customized replaceEqualDeep

[TkDodo]

Is there a better way to customize the replaceEqualDeep function?

use superjson

[tran-simon]

superjson is much slower than replaceEqualDeep.

I don't think it makes sense to add another dependency, when all we want is to add a few line to the built-in replaceEqualDeep

replaceEqualDeep could be refactored to accept a third parameter that is used as a callback function ran before the other comparisons. Or we could create a factory function that returns a custom replaceEqualDeep implementation

[TkDodo]

then just copy paste our code into your own, it's fine. Or don't send dates from the backend but serialize it to a ISO-8601 string and parse it again on the frontend.

when all we want is to add a few line to the built-in replaceEqualDeep

Really, we're not going to go the route of starting to serialize non json data. I hope I made that clear. You're free to disagree with that of course, and you have multiple options to work around this limitation.

[gaearon]

This may be very tangential but I wanted to note that RSC has built-in support for more things than JSON — partially inspired by what the structuredClone standard supports. In particular, Dates, Maps, and Sets are supported.

Although RQ by itself is unrelated to RSC, for people using the two together, it won't be uncommon to plumb some dates down from the server — given that they can serialize well and are unambiguous (and RSC supports it). So it's a bit unfortunate that RQ becomes a limiting factor here breaking a default optimization. It's easy to miss why it broke without very targeted debugging. Will become more of an issue as passing dates down becomes more common.

Another case in which we'd bump into that is custom SDKs that might include dates in their hydrated response objects. It's fair to say that these might be too custom but that's how we accidentally regressed on this (bluesky-social/social-app#3785 is the fix). A date crept in, the code kept working, the optimization broke.

So I wanted to voice support for raising the bar from JSON-serializable to some superset. Of course you'd have to choose which superset, and there could be an endless debate where nobody goes away happy. But you could consider a pragmatic subset of Structured Clone, maybe a narrower one than React did. Curiously, RQ is already technically doing that because its comparison algorithm already handles undefined, null, Infinity, NaN (oops, probably not due to ===), -0, and BigInts — none of which are JSON-serializable (but are in Structured Clone).

I think that makes the argument for adding Dates as well (without a bunch of other logic) a bit stronger. IMO it would be a significant improvement, would not compromise the code size or complexity, and maybe doesn't have to be a slippery slope.

Thanks for considering!

[gaearon]

Proposal.

We could start with Structured Clone JavaScript Types and narrow it down.

Let's exclude a few for a similar reason React excluded them pragmatically:

• Wrapper objects like Boolean, Number, String — too exotic, no real use case.
• RegExp — possible security implications for passing down.

Then let's exclude binary and low-level stuff:

• ArrayBuffer
• DataView
• TypedArray

We're left with Array (already supported), Date (needs support), Map and Set (need support), and NaN (needs support). So it's not a lot of code and the list is very limited and pragmatically bounded.

[TkDodo]

Thanks for taking the time to post these details. We currently have multiple places where we say that we need json serializable data:

• for structuralSharing to work (as you've noticed)
• for hashing the queryKey
• for our persistence API, that is used for writing data to / reading data from storages (like localstorage)

All of these places can be customized:

• for structuralSharing, you can pass your own function to it.
• for hashing the queryKey, you can set a queryKeyHashFn
• for persistence, you can pass serialize and deserialize to createPersister or createSyncStoragePersister / createASyncStoragePersister.

If we draw the line at Date, Map, Set and NaN it feels a bit arbitrary (though the structuredClone reference makes sense), and I fully expect people to ask us about "but why no Regex then?". It's much easier to argue along the lines of "only JSON is allowed, everything else, you're on your own".

I usually recommend superjson because they provide a thin wrapper around stringify / parse that can cover most if not all of the cases in user-land.

I can see that this is difficult for structuralSharing per se, because you'd need to swap out everything and do other field comparisons, too. Maybe there's something we can do better here, I'll think about it.