[SDK] Filter out AGW from searchable wallets #8062
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: abe9f3b 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 |
WalkthroughCentralized non-searchable wallet filtering via a new exported constant ( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant UI as Wallets UI (web/native)
participant F as Filter Logic
participant C as NON_SEARCHABLE_WALLETS
participant S as Search/Fuse
participant D as Display
U->>UI: Open wallets modal / list
UI->>F: Prepare wallet candidates
F->>C: Exclude IDs in NON_SEARCHABLE_WALLETS
F-->>UI: Filtered wallets
U->>UI: Type search query
UI->>S: Apply search on filtered set
S-->>UI: Matching wallets
UI->>D: Render results
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ 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). (5)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Comment |
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. This stack of pull requests is managed by Graphite. Learn more about stacking. |
![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 (2)
packages/thirdweb/src/react/native/ui/connect/ExternalWalletsList.tsx (1)
165-170: Remove unnecessary cast and type the memo output.Avoid
as WalletIdby typing the memo result.- const walletsToShow = useMemo(() => { + const walletsToShow: WalletId[] = useMemo(() => { @@ - wallet={createWallet(walletId as WalletId)} + wallet={createWallet(walletId)}packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/AllWalletsUI.tsx (1)
41-45: Optional: Set-based lookup to reduce per-item work.Tiny perf nit if the wallet list grows: memoize a Set and use
.has.- const walletList = useMemo(() => { - return walletInfos.filter( - (info) => !NON_SEARCHABLE_WALLETS.includes(info.id), - ); - }, []); + const nonSearchable = useMemo( + () => new Set(NON_SEARCHABLE_WALLETS), + [], + ); + const walletList = useMemo(() => { + return walletInfos.filter((info) => !nonSearchable.has(info.id)); + }, [nonSearchable]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.changeset/wide-flies-raise.md(1 hunks)packages/thirdweb/src/react/native/ui/connect/ExternalWalletsList.tsx(2 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/AllWalletsUI.tsx(2 hunks)packages/thirdweb/src/wallets/constants.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
.changeset/*.md: Each change inpackages/*must include a changeset for the appropriate package
Version bump rules: patch for non‑API changes; minor for new/modified public API
Files:
.changeset/wide-flies-raise.md
**/*.{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
**/*.{ts,tsx}: Use explicit function declarations and explicit return types in TypeScript
Limit each file to one stateless, single‑responsibility function
Re‑use shared types from@/typeswhere applicable
Prefertypealiases overinterfaceexcept for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Prefer composition over inheritance; use utility types (Partial, Pick, etc.)
Lazy‑import optional features and avoid top‑level side‑effects to reduce bundle size
Files:
packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/AllWalletsUI.tsxpackages/thirdweb/src/wallets/constants.tspackages/thirdweb/src/react/native/ui/connect/ExternalWalletsList.tsx
**/*.{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/react/web/ui/ConnectWallet/Modal/AllWalletsUI.tsxpackages/thirdweb/src/wallets/constants.tspackages/thirdweb/src/react/native/ui/connect/ExternalWalletsList.tsx
packages/thirdweb/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/thirdweb/**/*.{ts,tsx}: Every public symbol must have comprehensive TSDoc with at least one compiling@exampleand a custom tag (@beta,@internal,@experimental, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
Lazy‑load heavy dependencies inside async paths (e.g.,const { jsPDF } = await import("jspdf"))
Files:
packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/AllWalletsUI.tsxpackages/thirdweb/src/wallets/constants.tspackages/thirdweb/src/react/native/ui/connect/ExternalWalletsList.tsx
packages/thirdweb/src/wallets/**
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/wallets/**: UnifiedWalletandAccountinterfaces in wallet architecture
Support for in-app wallets (social/email login)
Smart wallets with account abstraction
EIP-1193, EIP-5792, EIP-7702 standard support in wallet modules
Files:
packages/thirdweb/src/wallets/constants.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : EIP-1193, EIP-5792, EIP-7702 standard support in wallet modules
📚 Learning: 2025-07-18T19:19:55.613Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : Smart wallets with account abstraction
Applied to files:
.changeset/wide-flies-raise.md
📚 Learning: 2025-07-18T19:19:55.613Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : Unified `Wallet` and `Account` interfaces in wallet architecture
Applied to files:
packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/AllWalletsUI.tsx
📚 Learning: 2025-07-18T19:20:32.530Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*client.tsx : Interactive UI that relies on hooks (`useState`, `useEffect`, React Query, wallet hooks).
Applied to files:
packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/AllWalletsUI.tsx
📚 Learning: 2025-07-18T19:19:55.613Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : Support for in-app wallets (social/email login)
Applied to files:
packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/AllWalletsUI.tsxpackages/thirdweb/src/react/native/ui/connect/ExternalWalletsList.tsx
📚 Learning: 2025-07-18T19:20:32.530Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*client.tsx : Anything that consumes hooks from `tanstack/react-query` or thirdweb SDKs.
Applied to files:
packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/AllWalletsUI.tsx
📚 Learning: 2025-06-17T18:30:52.976Z
Learnt from: MananTank
PR: thirdweb-dev/js#7356
File: apps/nebula/src/app/not-found.tsx:1-1
Timestamp: 2025-06-17T18:30:52.976Z
Learning: In the thirdweb/js project, the React namespace is available for type annotations (like React.FC) without needing to explicitly import React. This is project-specific configuration that differs from typical TypeScript/React setups.
Applied to files:
packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/AllWalletsUI.tsx
📚 Learning: 2025-07-18T19:20:32.530Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*client.tsx : When you need access to browser APIs (localStorage, window, IntersectionObserver etc.).
Applied to files:
packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/AllWalletsUI.tsx
📚 Learning: 2025-07-18T19:19:55.613Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to packages/thirdweb/src/wallets/** : EIP-1193, EIP-5792, EIP-7702 standard support in wallet modules
Applied to files:
packages/thirdweb/src/wallets/constants.tspackages/thirdweb/src/react/native/ui/connect/ExternalWalletsList.tsx
📚 Learning: 2025-07-18T19:19:55.613Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T19:19:55.613Z
Learning: Applies to test/src/test-wallets.ts : Predefined test accounts are in `test/src/test-wallets.ts`
Applied to files:
packages/thirdweb/src/wallets/constants.tspackages/thirdweb/src/react/native/ui/connect/ExternalWalletsList.tsx
🧬 Code graph analysis (3)
packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/AllWalletsUI.tsx (1)
packages/thirdweb/src/wallets/constants.ts (1)
NON_SEARCHABLE_WALLETS(10-10)
packages/thirdweb/src/wallets/constants.ts (1)
packages/thirdweb/src/exports/wallets.ts (1)
WalletId(139-139)
packages/thirdweb/src/react/native/ui/connect/ExternalWalletsList.tsx (1)
packages/thirdweb/src/wallets/constants.ts (1)
NON_SEARCHABLE_WALLETS(10-10)
⏰ 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). (7)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Unit Tests
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
packages/thirdweb/src/wallets/constants.ts (2)
1-1: Type import is correct.Local type import keeps bundle clean; no side effects.
10-10: Verify wallet id "xyz.abs" maps to a generated wallet-infos entry.rg failed because packages/thirdweb/src/wallets/generated/wallet-infos.js was not found — confirm that an entry with id: "xyz.abs" exists in the generated wallet-infos (check generated/wallet-infos.{js,ts}) and that it maps to the intended AGW entry; if missing or incorrect, remove or correct "xyz.abs" from NON_SEARCHABLE_WALLETS (packages/thirdweb/src/wallets/constants.ts:10).
packages/thirdweb/src/react/native/ui/connect/ExternalWalletsList.tsx (1)
99-107: LGTM: centralized filter via NON_SEARCHABLE_WALLETS.Removes hard-coded ids; behavior now follows a single source of truth.
packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/AllWalletsUI.tsx (2)
41-45: LGTM: reuse NON_SEARCHABLE_WALLETS in web modal.Consistent with native; simple and correct.
1-1: No issues with client directive and imports.Import paths and hook usage remain idiomatic for this codebase.
.changeset/wide-flies-raise.md (1)
1-6: ```shell
#!/bin/bash
set -euo pipefailSearch for references to AGW and related tokens within packages/thirdweb without relying on rg file-type flags
rg -n -C2 -g 'packages/thirdweb/**/*.{md,ts,tsx,js,jsx}' '\b(inApp|embedded|smart|AGW|NON_SEARCHABLE_WALLETS)\b' || true
Fallback: search entire repo for the same tokens (in case files are outside packages/thirdweb)
rg -n -C2 -g '**/*.{md,ts,tsx,js,jsx}' '\b(inApp|embedded|smart|AGW|NON_SEARCHABLE_WALLETS)\b' || true
Show the changeset file referenced in the review for context
echo "=== .changeset/wide-flies-raise.md ==="
if [ -f ".changeset/wide-flies-raise.md" ]; then
sed -n '1,240p' .changeset/wide-flies-raise.md || true
else
echo "(file not found)"
fiAlso look for a shared constant name that the comment mentions (NON_SEARCHABLE_WALLETS)
rg -n -C2 -g 'packages/**/*' 'NON_SEARCHABLE_WALLETS' || true
</blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| // Wallet IDs that should not appear in searchable wallet lists | ||
| export const NON_SEARCHABLE_WALLETS: WalletId[] = ["inApp", "embedded", "smart", "xyz.abs"] as const; |
There was a problem hiding this comment.
Fix TypeScript readonly mismatch and add required TSDoc.
as const creates a readonly tuple which is not assignable to WalletId[]. Also, public symbols under packages/thirdweb require TSDoc with a custom tag.
Apply:
-// Wallet IDs that should not appear in searchable wallet lists
-export const NON_SEARCHABLE_WALLETS: WalletId[] = ["inApp", "embedded", "smart", "xyz.abs"] as const;
+/**
+ * Wallet IDs that should not appear in searchable wallet lists.
+ * @internal
+ * @example
+ * // filter out from a list of wallet infos
+ * walletInfos.filter((info) => !NON_SEARCHABLE_WALLETS.includes(info.id));
+ */
+export const NON_SEARCHABLE_WALLETS =
+ ["inApp", "embedded", "smart", "xyz.abs"] as const
+ satisfies readonly WalletId[];Optional micro‑opt (use where hot): export a Set for O(1) lookups.
+/** @internal */
+export const NON_SEARCHABLE_WALLETS_SET = new Set<string>(NON_SEARCHABLE_WALLETS);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Wallet IDs that should not appear in searchable wallet lists | |
| export const NON_SEARCHABLE_WALLETS: WalletId[] = ["inApp", "embedded", "smart", "xyz.abs"] as const; | |
| /** | |
| * Wallet IDs that should not appear in searchable wallet lists. | |
| * @internal | |
| * @example | |
| * // filter out from a list of wallet infos | |
| * walletInfos.filter((info) => !NON_SEARCHABLE_WALLETS.includes(info.id)); | |
| */ | |
| export const NON_SEARCHABLE_WALLETS = | |
| ["inApp", "embedded", "smart", "xyz.abs"] as const | |
| satisfies readonly WalletId[]; | |
| /** @internal */ | |
| export const NON_SEARCHABLE_WALLETS_SET = new Set<string>(NON_SEARCHABLE_WALLETS); |
🤖 Prompt for AI Agents
In packages/thirdweb/src/wallets/constants.ts lines 9-10, the declaration uses
"as const" which produces a readonly tuple incompatible with WalletId[] and the
exported symbol lacks the required TSDoc with the project-specific tag; change
the exported constant to a type-compatible readonly array (e.g., declare it as
readonly WalletId[] or plain WalletId[] and remove "as const") and add the
required TSDoc block above the export including the custom tag used by our repo;
optionally also export a Set built from that array (e.g.,
NON_SEARCHABLE_WALLETS_SET) for O(1) lookups.
size-limit report 📦
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8062 +/- ##
==========================================
+ Coverage 56.51% 56.54% +0.02%
==========================================
Files 904 904
Lines 58865 58871 +6
Branches 4170 4170
==========================================
+ Hits 33269 33287 +18
+ Misses 25491 25478 -13
- Partials 105 106 +1
🚀 New features to boost your workflow:
|
joaquim-verges
force-pushed
the
_SDK_Filter_out_AGW_from_searchable_wallets
branch
from
11651e3 to
abe9f3b
Compare
joaquim-verges
deleted the
_SDK_Filter_out_AGW_from_searchable_wallets
branch
PR-Codex overview
This PR focuses on enhancing wallet search functionality by filtering out specific non-searchable wallets from the lists displayed in the UI.
Detailed summary
NON_SEARCHABLE_WALLETSconstant inpackages/thirdweb/src/wallets/constants.tsto define wallets that should not appear in searchable lists.walletListinpackages/thirdweb/src/react/web/ui/ConnectWallet/Modal/AllWalletsUI.tsxto useNON_SEARCHABLE_WALLETSfor filtering.packages/thirdweb/src/react/native/ui/connect/ExternalWalletsList.tsxto utilizeNON_SEARCHABLE_WALLETS.Summary by CodeRabbit
Bug Fixes
Refactor
Chores