handle Symbol properties and non-enumerable properties in `replaceEqualDeep` by phryneas · Pull Request #4237 · TanStack/router

phryneas · 2025-05-23T10:45:09Z

Opening this PR for discussion, I'm very open to scope changes.

This PR tackles two problems with defaultStructuralSharing: true:

symbol properties are not taken into account for "object equality" and are not copied over when creating a new copy
non-enumerable properties are not taken into account for "object equality" and are not copied over when creating a new copy

Point 1. actually is causing some of our users a headache, see apollographql/apollo-client#12619 and https://community.apollographql.com/t/preloadquery-with-tanstack-router-loader/9100 .

Users expect that they're able to pass an object with symbol properties over from a loader into components, but when having defaultStructuralSharing enabled, from the second object that is passed over, it creates a copy and strips off symbol properties.
That new object without the expected symbol properties will then crash our useReadQuery hook.

In other words, the first loader call returns

{
    refObject: { toPromise: Function1, [secretSymbol]: ImplementationDetail1 }
}

the component receives

{
    refObject: { toPromise: Function1, [secretSymbol]: ImplementationDetail1 }
}

and the second loader call returns

{
    refObject: { toPromise: Function2, [secretSymbol]: ImplementationDetail2 }
}

but the component receives

{
    refObject: { toPromise: Function2 }
}

toPromise is a different function, so it goes into the "clone" path, but skips the secretSymbol property.

If we wouldn't have that toPromise property on there (and we're considering removing it!), it would even be worse to debug - users would always stay on the first refObject and never get the second, since both objects would be considered equal.

On our side, passing these objects from loaders to components is actually an intended pattern, so this is causing quite a bit of problems - see this example from our docs (see Initiating queries outside React):

import { useLoaderData } from 'react-router-dom';

export function loader() {
  return preloadQuery(GET_DOGS_QUERY);
}

export function RouteComponent() {
  const queryRef = useLoaderData();
  const { data } = useReadQuery(queryRef);

  return (
    // ...
  );
}

Problem 2. is purely hypothetical and we haven't encountered it - but it should probably be handled in some way.

Both of these problems can be handled in two ways:

take these "special properties" into account for comparisons and copy them over (my naive approach here would lose the "non-enumerability" of these properties, more code would be necessary to keep that)
in isPlainObject just return false if an object has non-enumerable or symbol properties. This would opt out of structural sharing for these objects (which is probably perfectly fine)

I'd be open for either of those solutions, and also for completely dropping the "non-enumerable" case for simplicity, but I would be very happy if we could get something in to help with symbol properties.
In React Query, these values can be assumed to be serializable JSON, so the current implementation is perfectly fine, but with client-side loaders, users can just pass anything, and here it's causing quite the problem.
Telling our users to situationally turn off structural sharing would be an educational nightmare.

…alDeep`

nx-cloud · 2025-05-23T10:45:40Z

View your CI Pipeline Execution ↗ for commit b248312.

Command	Status	Duration	Result
`nx affected --targets=test:eslint,test:unit,tes...`	✅ Succeeded	2m 50s	View ↗
`nx run-many --target=build --exclude=examples/*...`	✅ Succeeded	3s	View ↗

☁️ Nx Cloud last updated this comment at 2025-05-23 12:26:18 UTC

pkg-pr-new · 2025-05-23T10:48:52Z

More templates

@tanstack/arktype-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/arktype-adapter@4237

@tanstack/directive-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/directive-functions-plugin@4237

@tanstack/history

npm i https://pkg.pr.new/TanStack/router/@tanstack/history@4237

@tanstack/eslint-plugin-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/eslint-plugin-router@4237

@tanstack/react-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router@4237

@tanstack/react-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-devtools@4237

@tanstack/react-router-with-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-router-with-query@4237

@tanstack/react-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start@4237

@tanstack/react-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-client@4237

@tanstack/react-start-config

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-config@4237

@tanstack/react-start-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-plugin@4237

@tanstack/react-start-router-manifest

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-router-manifest@4237

@tanstack/react-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/react-start-server@4237

@tanstack/router-cli

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-cli@4237

@tanstack/router-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-core@4237

@tanstack/router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools@4237

@tanstack/router-devtools-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-devtools-core@4237

@tanstack/router-generator

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-generator@4237

@tanstack/router-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-plugin@4237

@tanstack/router-utils

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-utils@4237

@tanstack/router-vite-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-vite-plugin@4237

@tanstack/server-functions-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/server-functions-plugin@4237

@tanstack/solid-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router@4237

@tanstack/solid-router-devtools

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-devtools@4237

@tanstack/solid-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start@4237

@tanstack/solid-start-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-client@4237

@tanstack/solid-start-config

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-config@4237

@tanstack/solid-start-plugin

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-plugin@4237

@tanstack/solid-start-router-manifest

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-router-manifest@4237

@tanstack/solid-start-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-start-server@4237

@tanstack/start

npm i https://pkg.pr.new/TanStack/router/@tanstack/start@4237

@tanstack/start-api-routes

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-api-routes@4237

@tanstack/start-client-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-client-core@4237

@tanstack/start-config

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-config@4237

@tanstack/start-server-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-core@4237

@tanstack/start-server-functions-client

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-functions-client@4237

@tanstack/start-server-functions-fetcher

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-functions-fetcher@4237

@tanstack/start-server-functions-server

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-functions-server@4237

@tanstack/start-server-functions-handler

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-functions-handler@4237

@tanstack/start-server-functions-ssr

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-server-functions-ssr@4237

@tanstack/valibot-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/valibot-adapter@4237

@tanstack/virtual-file-routes

npm i https://pkg.pr.new/TanStack/router/@tanstack/virtual-file-routes@4237

@tanstack/zod-adapter

npm i https://pkg.pr.new/TanStack/router/@tanstack/zod-adapter@4237

commit: b248312

phryneas · 2025-05-23T12:20:40Z

packages/router-core/tests/utils.test.ts

+      // without this PR:
+      // expect(result).toStrictEqual({ a: 3 })
+      // with this PR:
+      expect(result).toStrictEqual(obj2)


without this PR:

expect(result).toStrictEqual({ a: 3 })

phryneas · 2025-05-23T12:20:56Z

packages/router-core/tests/utils.test.ts

+      // without this PR:
+      // expect(result).toBe(obj1)
+      // with this PR:
+      expect(result).toStrictEqual(obj2)


without this PR:

expect(result).toBe(obj1)

phryneas · 2025-05-23T12:21:13Z

packages/router-core/tests/utils.test.ts

+      // expect(result).toBe(obj1)
+      // expect(result.b).toBe(2)
+      // with this PR:
+      expect(result).toBe(obj2)


without this PR:

expect(result).toBe(obj1) expect(result.b).toBe(2)

phryneas · 2025-05-23T12:21:28Z

packages/router-core/tests/utils.test.ts

+      // expect(result).toStrictEqual({ a: 3 })
+      // expect(result.b).toBe(undefined)
+      // with this PR:
+      expect(result).toBe(obj2)


without this PR:

expect(result).toStrictEqual({ a: 3 }) expect(result.b).toBe(undefined)

## Description Improve performance of this function that gets called a lot for structural sharing. Main improvements are - avoid creating object when possible (`Object.keys().concat(Object.getOwnPropertySymbols())` creates 3 arrays, when we only want 1) - instantiating array to the correct size avoids a lot of memory management under the hood (prefer `new Array(size)` over `[]`) - avoid reading the same value many times on an object, store is as a const More minor changes (not 100% sure I can measure it, but I think so): - using `keys.includes(k)` is slower than `Object.hasOwnProperty.call(obj, k)` or `obj.hasOwnProperty(k)` ## benchmark TL;DR: consistently 1.3x faster implementation ```ts describe('replaceEqualDeep', () => { bench('old implementation', () => { replaceEqualDeepOld({ a: 1, b: [2, 3] }, { a: 1, b: [2, 3] }) replaceEqualDeepOld({ a: 1, b: [2, 3] }, { a: 2, b: [2] }) }) bench('new implementation', () => { replaceEqualDeep({ a: 1, b: [2, 3] }, { a: 1, b: [2, 3] }) replaceEqualDeep({ a: 1, b: [2, 3] }, { a: 2, b: [2] }) }) }) ``` ```sh ✓ @tanstack/router-core tests/utils.bench.ts > replaceEqualDeep 1540ms name hz min max mean p75 p99 p995 p999 rme samples · old implementation 1,040,201.62 0.0008 0.7638 0.0010 0.0010 0.0013 0.0016 0.0022 ±0.33% 520101 · new implementation 1,347,988.70 0.0006 2.4037 0.0007 0.0007 0.0010 0.0010 0.0013 ±0.95% 673995 fastest BENCH Summary @tanstack/router-core new implementation - tests/utils.bench.ts > replaceEqualDeep 1.30x faster than old implementation ``` --- The `replaceEqualDeep` implementation before this PR handles Symbol keys, and non-enumerable keys (see #4237), but **not** keys that are both a Symbol and non-enumerable. This PR fixes this issue. But if we also fix it in the previous implementation before comparing performance we get a bigger perf diff ```sh ✓ @tanstack/router-core tests/utils.bench.ts > replaceEqualDeep 1471ms name hz min max mean p75 p99 p995 p999 rme samples · old implementation 713,964.88 0.0012 0.7880 0.0014 0.0014 0.0019 0.0023 0.0050 ±0.35% 356983 · new implementation 1,319,003.07 0.0006 5.0000 0.0008 0.0007 0.0010 0.0016 0.0050 ±1.96% 659502 fastest BENCH Summary @tanstack/router-core new implementation - tests/utils.bench.ts > replaceEqualDeep 1.85x faster than old implementation ``` ---  ## Summary by CodeRabbit * **Bug Fixes** * Change detection now includes symbol-keyed properties, treats undefined/missing entries consistently, and avoids processing objects with non-enumerable own properties to prevent incorrect updates. * No public API changes. * **Performance** * Equality/merge logic reuses unchanged values more reliably and improves array update handling by allocating appropriately, reducing unnecessary work.  --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

## Description Improve performance of this function that gets called a lot for structural sharing. Main improvements are - avoid creating object when possible (`Object.keys().concat(Object.getOwnPropertySymbols())` creates 3 arrays, when we only want 1) - instantiating array to the correct size avoids a lot of memory management under the hood (prefer `new Array(size)` over `[]`) - avoid reading the same value many times on an object, store is as a const More minor changes (not 100% sure I can measure it, but I think so): - using `keys.includes(k)` is slower than `Object.hasOwnProperty.call(obj, k)` or `obj.hasOwnProperty(k)` ## benchmark TL;DR: consistently 1.3x faster implementation ```ts describe('replaceEqualDeep', () => { bench('old implementation', () => { replaceEqualDeepOld({ a: 1, b: [2, 3] }, { a: 1, b: [2, 3] }) replaceEqualDeepOld({ a: 1, b: [2, 3] }, { a: 2, b: [2] }) }) bench('new implementation', () => { replaceEqualDeep({ a: 1, b: [2, 3] }, { a: 1, b: [2, 3] }) replaceEqualDeep({ a: 1, b: [2, 3] }, { a: 2, b: [2] }) }) }) ``` ```sh ✓ @tanstack/router-core tests/utils.bench.ts > replaceEqualDeep 1540ms name hz min max mean p75 p99 p995 p999 rme samples · old implementation 1,040,201.62 0.0008 0.7638 0.0010 0.0010 0.0013 0.0016 0.0022 ±0.33% 520101 · new implementation 1,347,988.70 0.0006 2.4037 0.0007 0.0007 0.0010 0.0010 0.0013 ±0.95% 673995 fastest BENCH Summary @tanstack/router-core new implementation - tests/utils.bench.ts > replaceEqualDeep 1.30x faster than old implementation ``` --- The `replaceEqualDeep` implementation before this PR handles Symbol keys, and non-enumerable keys (see TanStack#4237), but **not** keys that are both a Symbol and non-enumerable. This PR fixes this issue. But if we also fix it in the previous implementation before comparing performance we get a bigger perf diff ```sh ✓ @tanstack/router-core tests/utils.bench.ts > replaceEqualDeep 1471ms name hz min max mean p75 p99 p995 p999 rme samples · old implementation 713,964.88 0.0012 0.7880 0.0014 0.0014 0.0019 0.0023 0.0050 ±0.35% 356983 · new implementation 1,319,003.07 0.0006 5.0000 0.0008 0.0007 0.0010 0.0016 0.0050 ±1.96% 659502 fastest BENCH Summary @tanstack/router-core new implementation - tests/utils.bench.ts > replaceEqualDeep 1.85x faster than old implementation ``` ---  ## Summary by CodeRabbit * **Bug Fixes** * Change detection now includes symbol-keyed properties, treats undefined/missing entries consistently, and avoids processing objects with non-enumerable own properties to prevent incorrect updates. * No public API changes. * **Performance** * Equality/merge logic reuses unchanged values more reliably and improves array update handling by allocating appropriately, reducing unnecessary work.  --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

handle Symbol properties and non-enumerable properties in `replaceEqu…

ee5d425

…alDeep`

github-actions bot added the package: router-core label May 23, 2025

phryneas mentioned this pull request May 23, 2025

Using toPromise in a tanstack router loader loses internal queryRef apollographql/apollo-client#12619

Closed

phryneas added 2 commits May 23, 2025 14:18

changes from feedback

fc864f8

remove comments

b248312

phryneas commented May 23, 2025

View reviewed changes

schiller-manuel requested a review from TkDodo May 23, 2025 18:08

TkDodo approved these changes May 24, 2025

View reviewed changes

schiller-manuel merged commit ef01673 into TanStack:main May 24, 2025
5 checks passed

jerelmiller mentioned this pull request Aug 28, 2025

[4.0] preloadQuery variables don't update when they change apollographql/apollo-client#12885

Closed

Sheraff mentioned this pull request Aug 30, 2025

refactor(router-core): slightly faster replaceEqualDeep #5046

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

handle Symbol properties and non-enumerable properties in `replaceEqualDeep`#4237

handle Symbol properties and non-enumerable properties in `replaceEqualDeep`#4237
schiller-manuel merged 3 commits intoTanStack:mainfrom
phryneas:pr/replaceEqualDeep-handle-symbol-and-non-enumerable

phryneas commented May 23, 2025 •

edited

Loading

Uh oh!

nx-cloud bot commented May 23, 2025 •

edited

Loading

Uh oh!

pkg-pr-new bot commented May 23, 2025 •

edited

Loading

Uh oh!

phryneas May 23, 2025

Uh oh!

phryneas May 23, 2025

Uh oh!

phryneas May 23, 2025

Uh oh!

phryneas May 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

phryneas commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nx-cloud bot commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pkg-pr-new bot commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phryneas May 23, 2025

Choose a reason for hiding this comment

Uh oh!

phryneas May 23, 2025

Choose a reason for hiding this comment

Uh oh!

phryneas May 23, 2025

Choose a reason for hiding this comment

Uh oh!

phryneas May 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

phryneas commented May 23, 2025 •

edited

Loading

nx-cloud bot commented May 23, 2025 •

edited

Loading

pkg-pr-new bot commented May 23, 2025 •

edited

Loading