feat: replace `JSON.stringify` with `replaceDeepEqual` in structural sharing integrity check #8030

jxom · 2024-09-08T11:34:17Z

This PR is a follow up on #7966 to:

~~not surface console errors for values that are referentially stable (e.g. BigInt)~~
~~detect circular references in replaceDeepEqual~~
~~catch errors from replaceDeepEqual in replaceData, and surface them in the console + propagated to the query.~~
replace JSON.stringify with replaceDeepEqual in structural sharing integrity check

nx-cloud · 2024-09-08T11:34:48Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit cd2c14c. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

jxom · 2024-09-08T11:38:07Z

packages/query-core/src/utils.ts

+      if (seen.has(a)) {
+        throw new Error('circular reference detected.')
+      }
+      seen.add(a)


these lines were added

jxom · 2024-09-08T11:38:17Z

packages/query-core/src/utils.ts

-  if (a === b) {
-    return a
-  }
+  const seen = new WeakSet()


this line was added

pkg-pr-new · 2024-09-08T11:39:12Z

Open in Stackblitz

More templates

@tanstack/eslint-plugin-query

pnpm add https://pkg.pr.new/@tanstack/eslint-plugin-query@8030

@tanstack/angular-query-experimental

pnpm add https://pkg.pr.new/@tanstack/angular-query-experimental@8030

@tanstack/query-async-storage-persister

pnpm add https://pkg.pr.new/@tanstack/query-async-storage-persister@8030

@tanstack/query-broadcast-client-experimental

pnpm add https://pkg.pr.new/@tanstack/query-broadcast-client-experimental@8030

@tanstack/query-core

pnpm add https://pkg.pr.new/@tanstack/query-core@8030

@tanstack/query-devtools

pnpm add https://pkg.pr.new/@tanstack/query-devtools@8030

@tanstack/query-persist-client-core

pnpm add https://pkg.pr.new/@tanstack/query-persist-client-core@8030

@tanstack/query-sync-storage-persister

pnpm add https://pkg.pr.new/@tanstack/query-sync-storage-persister@8030

@tanstack/react-query

pnpm add https://pkg.pr.new/@tanstack/react-query@8030

@tanstack/react-query-devtools

pnpm add https://pkg.pr.new/@tanstack/react-query-devtools@8030

@tanstack/react-query-next-experimental

pnpm add https://pkg.pr.new/@tanstack/react-query-next-experimental@8030

@tanstack/react-query-persist-client

pnpm add https://pkg.pr.new/@tanstack/react-query-persist-client@8030

@tanstack/solid-query

pnpm add https://pkg.pr.new/@tanstack/solid-query@8030

@tanstack/solid-query-devtools

pnpm add https://pkg.pr.new/@tanstack/solid-query-devtools@8030

@tanstack/solid-query-persist-client

pnpm add https://pkg.pr.new/@tanstack/solid-query-persist-client@8030

@tanstack/svelte-query

pnpm add https://pkg.pr.new/@tanstack/svelte-query@8030

@tanstack/svelte-query-devtools

pnpm add https://pkg.pr.new/@tanstack/svelte-query-devtools@8030

@tanstack/svelte-query-persist-client

pnpm add https://pkg.pr.new/@tanstack/svelte-query-persist-client@8030

@tanstack/vue-query

pnpm add https://pkg.pr.new/@tanstack/vue-query@8030

@tanstack/vue-query-devtools

pnpm add https://pkg.pr.new/@tanstack/vue-query-devtools@8030

@tanstack/angular-query-devtools-experimental

pnpm add https://pkg.pr.new/@tanstack/angular-query-devtools-experimental@8030

commit: cd2c14c

codecov · 2024-09-08T11:41:42Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.85%. Comparing base (d58cf08) to head (cd2c14c).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##           main    #8030       +/-   ##
=========================================
+ Coverage      0   61.85%   +61.85%     
=========================================
  Files         0      135      +135     
  Lines         0     4679     +4679     
  Branches      0     1306     +1306     
=========================================
+ Hits          0     2894     +2894     
- Misses        0     1543     +1543     
- Partials      0      242      +242

Components	Coverage Δ
@tanstack/angular-query-devtools-experimental	`∅ <ø> (∅)`
@tanstack/angular-query-experimental	`86.58% <ø> (∅)`
@tanstack/eslint-plugin-query	`∅ <ø> (∅)`
@tanstack/query-async-storage-persister	`43.85% <ø> (∅)`
@tanstack/query-broadcast-client-experimental	`∅ <ø> (∅)`
@tanstack/query-codemods	`∅ <ø> (∅)`
@tanstack/query-core	`92.86% <100.00%> (∅)`
@tanstack/query-devtools	`4.86% <ø> (∅)`
@tanstack/query-persist-client-core	`57.73% <ø> (∅)`
@tanstack/query-sync-storage-persister	`82.50% <ø> (∅)`
@tanstack/react-query	`92.50% <ø> (∅)`
@tanstack/react-query-devtools	`10.00% <ø> (∅)`
@tanstack/react-query-next-experimental	`∅ <ø> (∅)`
@tanstack/react-query-persist-client	`100.00% <ø> (∅)`
@tanstack/solid-query	`78.20% <ø> (∅)`
@tanstack/solid-query-devtools	`∅ <ø> (∅)`
@tanstack/solid-query-persist-client	`100.00% <ø> (∅)`
@tanstack/svelte-query	`87.33% <ø> (∅)`
@tanstack/svelte-query-devtools	`∅ <ø> (∅)`
@tanstack/svelte-query-persist-client	`100.00% <ø> (∅)`
@tanstack/vue-query	`71.51% <ø> (∅)`
@tanstack/vue-query-devtools	`∅ <ø> (∅)`

TkDodo

I think we had a bit of a miscommunication. Ideally, we don't want any runtime changes for production builds - that's why the initial code was behind an env check.

So my idea was something like this:

if (process.env.NODE_ENV !== 'production') {
  try {
    return replaceEqualDeep(prevData, data)
  }
  catch(error) {
    console.error(...)
  }
}
return replaceEqualDeep(prevData, data)

This will remove the dev mode overhead of json stringify. If everything goes right, replaceEqualDeep will only be called once. If there is a circularity, the first call will log the error and the second call will then throw, so it will be caught in setData and the query will go to error state.

I think this would be the best of both worlds.

jxom · 2024-09-08T11:49:15Z

packages/query-core/src/utils.ts

+      // Structurally share data between prev and new data if needed
+      return replaceEqualDeep(prevData, data)
+    } catch (error) {
+      console.error(


Realised this should be under a dev env guard. Will fix when I’m back at computer.

jxom · 2024-09-08T11:53:33Z

I think we had a bit of a miscommunication. Ideally, we don't want any runtime changes for production builds - that's why the initial code was behind an env check.

So my idea was something like this:
if (process.env.NODE_ENV !== 'production') {
  try {
    return replaceEqualDeep(prevData, data)
  }
  catch(error) {
    console.error(...)
  }
}
return replaceEqualDeep(prevData, data)
This will remove the dev mode overhead of json stringify. If everything goes right, replaceEqualDeep will only be called once. If there is a circularity, the first call will log the error and the second call will then throw, so it will be caught in setData and the query will go to error state.

I think this would be the best of both worlds.

Got it. Can do that!

jxom · 2024-09-08T21:00:39Z

@TkDodo - lmk what you think

packages/query-core/src/utils.ts

feat: throw when replaceDeepEqual contains circular references

73c1752

jxom mentioned this pull request Sep 8, 2024

fix(query-core): handle errors that occur inside setData method #7966

Merged

github-actions bot added the package: query-core label Sep 8, 2024

autofix-ci bot and others added 2 commits September 8, 2024 11:35

ci: apply automated fixes

a610e3f

chore: up test

2bcece2

jxom commented Sep 8, 2024

View reviewed changes

packages/query-core/src/utils.ts Outdated

if (a === b) {

return a

}

const seen = new WeakSet()

Copy link

Contributor Author

jxom Sep 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line was added

TkDodo requested changes Sep 8, 2024

View reviewed changes

jxom commented Sep 8, 2024

View reviewed changes

chore: review changes

8de5540

jxom changed the title ~~feat: throw when replaceDeepEqual contains circular references~~ feat: replace JSON.stringify with replaceEqualDeep in structural sharing integrity check Sep 8, 2024

ci: apply automated fixes

93b2d56

jxom changed the title ~~feat: replace JSON.stringify with replaceEqualDeep in structural sharing integrity check~~ feat: replace JSON.stringify with replaceDeepEqual in structural sharing integrity check Sep 8, 2024

jxom mentioned this pull request Sep 8, 2024

Latest wagmi and @tanstack/react-query are not compatible with StructuralSharing (breaks dev) wevm/wagmi#4233

Closed

1 task

akshatmittal reviewed Sep 8, 2024

View reviewed changes

packages/query-core/src/utils.ts Outdated Show resolved Hide resolved

jxom and others added 2 commits September 9, 2024 07:36

chore: add return

9fc0c0a

Merge branch 'main' into jxom/cyclic-check

cd2c14c

TkDodo approved these changes Sep 9, 2024

View reviewed changes

TkDodo merged commit 1bffc78 into TanStack:main Sep 9, 2024
8 checks passed

jxom deleted the jxom/cyclic-check branch September 13, 2024 07:29

This was referenced Oct 3, 2024

[Snyk] Upgrade @tanstack/react-query-persist-client from 5.55.2 to 5.56.1 zntb/crypto-ticker#61

Merged

[Snyk] Upgrade @tanstack/query-sync-storage-persister from 5.55.2 to 5.56.2 zntb/crypto-ticker#62

Closed

jwinr mentioned this pull request Oct 6, 2024

[Snyk] Upgrade @tanstack/react-query from 5.55.0 to 5.56.2 jwinr/threadly#9

Merged

WontonSam mentioned this pull request Oct 14, 2024

[Snyk] Upgrade @tanstack/react-query from 4.36.1 to 5.56.2 WontonSam/jsoncrack.com#23

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: replace `JSON.stringify` with `replaceDeepEqual` in structural sharing integrity check #8030

feat: replace `JSON.stringify` with `replaceDeepEqual` in structural sharing integrity check #8030

jxom commented Sep 8, 2024 •

edited

Loading

nx-cloud bot commented Sep 8, 2024 •

edited

Loading

jxom Sep 8, 2024

jxom Sep 8, 2024

pkg-pr-new bot commented Sep 8, 2024 •

edited

Loading

codecov bot commented Sep 8, 2024 •

edited

Loading

TkDodo left a comment

jxom Sep 8, 2024

jxom commented Sep 8, 2024

jxom commented Sep 8, 2024

feat: replace JSON.stringify with replaceDeepEqual in structural sharing integrity check #8030

feat: replace JSON.stringify with replaceDeepEqual in structural sharing integrity check #8030

Conversation

jxom commented Sep 8, 2024 • edited Loading

nx-cloud bot commented Sep 8, 2024 • edited Loading

☁️ Nx Cloud Report

jxom Sep 8, 2024

Choose a reason for hiding this comment

jxom Sep 8, 2024

Choose a reason for hiding this comment

pkg-pr-new bot commented Sep 8, 2024 • edited Loading

codecov bot commented Sep 8, 2024 • edited Loading

Codecov Report

TkDodo left a comment

Choose a reason for hiding this comment

jxom Sep 8, 2024

Choose a reason for hiding this comment

jxom commented Sep 8, 2024

jxom commented Sep 8, 2024

feat: replace `JSON.stringify` with `replaceDeepEqual` in structural sharing integrity check #8030

feat: replace `JSON.stringify` with `replaceDeepEqual` in structural sharing integrity check #8030

jxom commented Sep 8, 2024 •

edited

Loading

nx-cloud bot commented Sep 8, 2024 •

edited

Loading

pkg-pr-new bot commented Sep 8, 2024 •

edited

Loading

codecov bot commented Sep 8, 2024 •

edited

Loading