[SDK] Fix: Deprecate bridge routes sortBy #7825
Conversation
🦋 Changeset detectedLatest commit: d216d76 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
WalkthroughDeprecation annotation added to the sortBy option in the bridge Routes options. A changeset is introduced to publish a patch release for the thirdweb package reflecting this deprecation. No runtime or control flow changes. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.changeset/shaky-candles-rule.md (1)
1-5: Strengthen the changeset with migration guidance and deprecation detailsAdd a short rationale, what to use instead (likely “remove the option; default sort is used”), and an estimated removal timeline. This helps downstream consumers and release notes readers.
Proposed tweak:
--- "thirdweb": patch --- -Deprecates `sortBy` parameter in Bridge.routes +Deprecate `Bridge.routes` option `sortBy`. + +Why: +- The API now determines sorting internally; external sorting is no longer supported. + +Impact: +- No runtime behavior change in this release. Passing `sortBy` continues to work but is deprecated. + +Migration: +- Remove the `sortBy` option from calls to `Bridge.routes`. The default ordering will be used. + +Deprecation plan: +- `sortBy` will be removed in a future release after a standard deprecation period.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/shaky-candles-rule.md(1 hunks)packages/thirdweb/src/bridge/Routes.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
Files:
packages/thirdweb/src/bridge/Routes.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
packages/thirdweb/src/bridge/Routes.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Size
- GitHub Check: Unit Tests
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Build Packages
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
| /** | ||
| * Sort routes by popularity | ||
| * @deprecated | ||
| */ |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add a deprecation message and align surrounding docs/examples
The @deprecated tag is good, but include a clear message and update earlier examples/param docs that still promote sortBy to avoid mixed signals.
Suggested JSDoc change:
/**
- * Sort routes by popularity
- * @deprecated
+ * Sort routes by popularity.
+ * @deprecated This option is deprecated and will be removed in a future release.
+ * Routes are now sorted by default by the API. Remove this option from your calls.
*/Follow-ups:
- Remove or annotate the example that uses
sortBy: "popularity"and update the@param options.sortBydescription above to reflect deprecation. - Optionally, consider a one-time runtime warning when
sortByis provided to aid discoverability (can be added behind a development flag).
Repo sweep to find usages:
🏁 Script executed:
#!/bin/bash
# Find references to Bridge.routes sortBy to update examples/tests/docs.
rg -n --no-heading --color=never $'Bridge\\.routes\\s*\\(|sortBy\\s*:\\s*"popularity"|options\\.sortBy' -A 2 -B 2Length of output: 9405
Action required: Add a clear deprecation message to Options.sortBy and update docs/tests/examples
Short summary:
- The JSDoc on Options.sortBy is marked @deprecated but has no message. Repository search shows many examples/tests/CHANGELOG entries still using
sortBy(includingsortBy: "popularity"). This will confuse users unless the deprecation is documented and references are updated.
Files/locations that need attention (from the repo scan):
- packages/thirdweb/src/bridge/Routes.ts
- Example blocks using
sortBy: "popularity"and the@param options.sortByJSDoc
- Example blocks using
- packages/thirdweb/CHANGELOG.md
- Historical sample code includes
sortBy: "popularity"(update or annotate)
- Historical sample code includes
- packages/thirdweb/src/react/core/hooks/useBridgeRoutes.test.ts
- Test sets
sortBy: "popularity"— update expected behavior or remove this option
- Test sets
- apps/portal/** (multiple .mdx pages)
- Examples in docs still pass
sortBy— update examples or annotate as deprecated
- Examples in docs still pass
- packages/thirdweb/src/react/core/hooks/transaction/useSendTransaction.ts
- Calls Bridge.routes — verify callers for
sortByusage
- Calls Bridge.routes — verify callers for
Suggested JSDoc change (apply to Options.sortBy JSDoc):
/**
- * Sort routes by popularity
- * @deprecated
+ * Sort routes by popularity.
+ * @deprecated This option is deprecated and will be removed in a future release.
+ * Routes are now ordered by the API by default — remove this option from your calls.
*/Recommended follow-up actions
- Update the JSDoc as shown above to include a deprecation message and migration guidance.
- Replace or annotate all examples in Routes.ts and .mdx docs that show
sortBy: "popularity". Prefer removing the option from examples and adding a short note that sorting is done server-side now. - Update tests that assert behavior with
sortBy:- Remove
sortByfrom fixtures/tests, or - Adjust tests to assert that providing
sortByhas no effect (if that’s the intended compatibility behavior).
- Remove
- Update CHANGELOG examples or annotate them as historical if you wish to preserve the snippet.
- Optional (recommended for discoverability): add a one-time dev-mode runtime warning when
options.sortByis provided, e.g.:
// inside the Bridge.routes implementation (dev-only)
if (options?.sortBy && process.env.NODE_ENV !== "production") {
const key = "__THIRDWEB_SORTBY_DEPRECATION_SHOWN";
if (!(globalThis as any)[key]) {
console.warn(
"[thirdweb] `options.sortBy` is deprecated and will be removed in a future release. The API sorts routes by default; remove this option from your calls."
);
(globalThis as any)[key] = true;
}
}Repo sweep (improved search to find remaining usages):
# Find literal usages of sortBy (skip node_modules/dist/build)
rg -n --no-heading --color=never --hidden --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' '\bsortBy\b' -SNotes:
- CHANGELOG entries are often historical; if you keep historical examples, annotate them so readers aren't misled.
- If you want to avoid runtime changes in this PR, at minimum update docs/tests to reflect the deprecation message and plan a follow-up PR for any runtime warnings or lint rules.
🤖 Prompt for AI Agents
packages/thirdweb/src/bridge/Routes.ts around lines 206-209: the JSDoc for
Options.sortBy is marked @deprecated but lacks any deprecation message or
migration guidance; update the JSDoc to include a clear deprecation message and
suggested migration (e.g., "deprecated — server now handles route sorting;
remove this option") and reference the replacement behavior, then scan and
update examples/tests/docs that still pass sortBy (remove the option or annotate
as deprecated) and adjust tests to either stop asserting on sortBy or assert it
has no effect; optionally add a dev-only runtime warning when options.sortBy is
provided to surface the deprecation during development.
size-limit report 📦
|
PR-Codex overview
This PR focuses on deprecating the
sortByparameter in theBridge.routesof thethirdwebpackage, indicating that it should no longer be used in future implementations.Detailed summary
sortByparameter inBridge.routes.sortByto include a@deprecatedtag, clarifying its status.Summary by CodeRabbit
New Features
Documentation
Chores