Skip to content

Conversation

@Sheraff
Copy link
Contributor

@Sheraff Sheraff commented Jan 21, 2026

This entire PR is basically just this one line: most objects going through replaceEqualDeep won't have symbols, so return early if that's the case:

if (symbols.length === 0) return names

Summary by CodeRabbit

  • Refactor

    • Optimized internal key-enumeration to reduce memory allocations for plain objects and improve handling of symbol keys.
  • Tests

    • Added comprehensive tests for key-enumeration covering symbol keys, non-enumerable properties, mixed scenarios, frozen/sealed objects, and typical router-state patterns.
    • Updated two test expectations to reflect a corrected update count during navigation.

✏️ Tip: You can customize this high-level summary in your review settings.

…key' common case, 6-11% faster replaceEqualDeep
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 21, 2026

📝 Walkthrough

Walkthrough

Reworks internal key-enumeration in router-core: adds an isEnumerable helper and a symbol-aware fast-path in getEnumerableOwnKeys to avoid allocations when no symbol properties exist; behavior remains externally unchanged.

Changes

Cohort / File(s) Summary
Core util change
packages/router-core/src/utils.ts
Add isEnumerable bound to Object.prototype.propertyIsEnumerable; implement fast-path in getEnumerableOwnKeys to return string keys directly when no symbols exist and merge string/symbol accumulation only when needed to reduce allocations.
Tests added
packages/router-core/tests/utils.test.ts
Add comprehensive tests covering string keys, symbol keys, mixed keys, non-enumerable/shadowed properties, prototype/inheritance edge cases, frozen/sealed/Object.create(null) objects, and router-like state scenarios.
Test expectation updates
packages/react-router/tests/store-updates-during-navigation.test.tsx
Two test expectations adjusted from 7 → 8 to match changed behavior/observation counts during navigation tests.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

performance

Suggested reviewers

  • schiller-manuel
  • nlynzaad

Poem

🐰 I nibble through keys, both string and sign,

fast-path footsteps — light and fine.
Symbols hide? I skip the chore,
fewer arrays, and hops galore.
Tests clap paws — the change is mine! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main optimization: a fast path in getEnumerableOwnKeys for the common case without symbol keys, resulting in measurable performance improvements (6-11% faster replaceEqualDeep).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

@nx-cloud
Copy link

nx-cloud bot commented Jan 21, 2026

View your CI Pipeline Execution ↗ for commit a459a7a

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

☁️ Nx Cloud last updated this comment at 2026-01-22 14:01:06 UTC

@pkg-pr-new
Copy link

pkg-pr-new bot commented Jan 21, 2026

More templates

@tanstack/arktype-adapter

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

@tanstack/eslint-plugin-router

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

@tanstack/history

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

@tanstack/nitro-v2-vite-plugin

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

@tanstack/react-router

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

@tanstack/react-router-devtools

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

@tanstack/react-router-ssr-query

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

@tanstack/react-start

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

@tanstack/react-start-client

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

@tanstack/react-start-server

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

@tanstack/router-cli

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

@tanstack/router-core

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

@tanstack/router-devtools

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

@tanstack/router-devtools-core

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

@tanstack/router-generator

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

@tanstack/router-plugin

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

@tanstack/router-ssr-query-core

npm i https://pkg.pr.new/TanStack/router/@tanstack/router-ssr-query-core@6448

@tanstack/router-utils

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

@tanstack/router-vite-plugin

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

@tanstack/solid-router

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

@tanstack/solid-router-devtools

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

@tanstack/solid-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/solid-router-ssr-query@6448

@tanstack/solid-start

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

@tanstack/solid-start-client

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

@tanstack/solid-start-server

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

@tanstack/start-client-core

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

@tanstack/start-fn-stubs

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-fn-stubs@6448

@tanstack/start-plugin-core

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

@tanstack/start-server-core

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

@tanstack/start-static-server-functions

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

@tanstack/start-storage-context

npm i https://pkg.pr.new/TanStack/router/@tanstack/start-storage-context@6448

@tanstack/valibot-adapter

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

@tanstack/virtual-file-routes

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

@tanstack/vue-router

npm i https://pkg.pr.new/TanStack/router/@tanstack/vue-router@6448

@tanstack/vue-router-devtools

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

@tanstack/vue-router-ssr-query

npm i https://pkg.pr.new/TanStack/router/@tanstack/vue-router-ssr-query@6448

@tanstack/vue-start

npm i https://pkg.pr.new/TanStack/router/@tanstack/vue-start@6448

@tanstack/vue-start-client

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

@tanstack/vue-start-server

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

@tanstack/zod-adapter

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

commit: a459a7a

@Sheraff Sheraff merged commit 850b963 into main Jan 22, 2026
6 checks passed
@Sheraff Sheraff deleted the refactor-router-core-replace-equal-deep-fast-path-no-symbols branch January 22, 2026 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants