Skip to content

[SDK] Fix: Deprecate bridge routes sortBy #7825

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 11, 2025
Merged

[SDK] Fix: Deprecate bridge routes sortBy #7825

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/shaky-candles-rule.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"thirdweb": patch
---

Deprecates `sortBy` parameter in Bridge.routes
5 changes: 4 additions & 1 deletion packages/thirdweb/src/bridge/Routes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,10 @@ export declare namespace routes {
destinationTokenAddress?: ox__Address.Address;
/** Transaction hash to filter routes by */
transactionHash?: ox__Hex.Hex;
/** Sort routes by popularity */
/**
* Sort routes by popularity
* @deprecated
*/
Comment on lines +206 to +209
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 11, 2025

Choose a reason for hiding this comment

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

💡 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.sortBy description above to reflect deprecation.
  • Optionally, consider a one-time runtime warning when sortBy is 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 2

Length 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 (including sortBy: "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.sortBy JSDoc
  • packages/thirdweb/CHANGELOG.md
    • Historical sample code includes sortBy: "popularity" (update or annotate)
  • packages/thirdweb/src/react/core/hooks/useBridgeRoutes.test.ts
    • Test sets sortBy: "popularity" — update expected behavior or remove this option
  • apps/portal/** (multiple .mdx pages)
    • Examples in docs still pass sortBy — update examples or annotate as deprecated
  • packages/thirdweb/src/react/core/hooks/transaction/useSendTransaction.ts
    • Calls Bridge.routes — verify callers for sortBy usage

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 sortBy from fixtures/tests, or
    • Adjust tests to assert that providing sortBy has no effect (if that’s the intended compatibility behavior).
  • 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.sortBy is 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' -S

Notes:

  • 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.

sortBy?: "popularity";
/** Maximum number of steps in the route */
maxSteps?: number;
Expand Down
Loading