perf(router-core): skip impossible JSON parse attempts - #8006
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe search parameter serializer now avoids impossible ChangesSearch serialization
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant stringifySearchWith
participant stringifyValue
participant JSON.parse
stringifySearchWith->>stringifyValue: serialize search value
stringifyValue->>stringifyValue: check jsonStart
stringifyValue->>JSON.parse: parse eligible string
JSON.parse-->>stringifyValue: parsed value
stringifyValue-->>stringifySearchWith: serialized value
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit e86a0d5
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version Preview1 package(s) bumped directly, 22 bumped as dependents. 🟩 Patch bumps
|
Bundle Size Benchmarks
The following scenarios have bundle-size changes compared with the baseline:
Current gzip tracks all emitted client JS chunks. Initial gzip tracks only the entry/import graph. Trend sparkline is historical current gzip ending with this PR measurement; lower is better. |
Merging this PR will degrade performance by 0.36%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Memory | mem server error-paths redirect (solid) |
588.2 KB | 288.5 KB | ×2 |
| ⚡ | Simulation | ssr request loop (react) |
80 ms | 51.8 ms | +54.26% |
| ⚡ | Simulation | client-search-params navigation loop (react) |
59.4 ms | 47.1 ms | +26.14% |
| ⚡ | Simulation | ssr rewrite localized (react) |
89 ms | 75.3 ms | +18.2% |
| ⚡ | Simulation | client-search-params navigation loop (vue) |
80.5 ms | 68.4 ms | +17.63% |
| ⚡ | Simulation | client-links navigation loop (react) |
90.6 ms | 77.2 ms | +17.41% |
| ⚡ | Simulation | client-search-params navigation loop (solid) |
114.8 ms | 99.9 ms | +14.93% |
| ⚡ | Simulation | ssr request loop (solid) |
158 ms | 144.4 ms | +9.41% |
| ⚡ | Simulation | ssr rewrite passthrough (react) |
114.1 ms | 104.6 ms | +9.08% |
| ⚡ | Simulation | client-links navigation loop (vue) |
163.9 ms | 150.5 ms | +8.92% |
| ⚡ | Simulation | ssr rewrite localized (solid) |
126.6 ms | 117 ms | +8.17% |
| ⚡ | Simulation | client-side navigation loop (react) |
41.2 ms | 38.1 ms | +8.05% |
| ⚡ | Simulation | ssr request loop (vue) |
270.2 ms | 254.9 ms | +6% |
| ⚡ | Memory | mem client preload-churn (vue) |
776.2 KB | 735.3 KB | +5.57% |
| ⚡ | Simulation | client-rewrites navigation loop (react) |
44.6 ms | 42.4 ms | +5.21% |
| ⚡ | Simulation | client-rewrites navigation loop (vue) |
48.1 ms | 45.9 ms | +4.77% |
| ⚡ | Simulation | client-side navigation loop (vue) |
70.1 ms | 67 ms | +4.6% |
| ⚡ | Simulation | client-side navigation loop (solid) |
88 ms | 84.4 ms | +4.26% |
| ⚡ | Simulation | ssr rewrite localized (vue) |
214.6 ms | 206.9 ms | +3.74% |
| ⚡ | Memory | mem server serialization-payload (solid) |
3.5 MB | 3.4 MB | +3.51% |
| ... | ... | ... | ... | ... | ... |
ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing agent/search-params-skip-json-parse (e86a0d5) with main (c568caf)
What changed
JSON.parsewhen a string's first character cannot begin valid JSONJSON.parseWhy
The default search serializer probes every string with
JSON.parseso JSON-looking strings remain strings after a round trip. Ordinary application strings usually fail that probe, making exception creation and handling the dominant cost. The grammar check avoids only parses that are guaranteed to fail.Impact
Focused local benchmarks, each serializing 1,000 objects:
react-router.minimalchanges from 85,919 to 85,956 bytes gzip (+37 bytes).Validation
@tanstack/router-core:test:unit: 105 files / 1,547 tests passed, plus 3 expected failures@tanstack/router-core:test:types: TypeScript 5.6 through 7.0 passed@tanstack/router-core:test:eslint: zero errors (existing warnings remain)tests/searchParams.bench.tsrunreact-router.minimalbundle-size scenario onmainand this branchgit diff --checkSummary by CodeRabbit
Bug Fixes
Performance
Tests