[Multi] Update dependencies, improve wagmi adapter #8167
Conversation
🦋 Changeset detectedLatest commit: 8580fe4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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 GitHub.
|
WalkthroughAdds a new Vite + React wagmi demo app and scaffolding; updates thirdweb internals (viem adapter, event log casting, gas authorization field) and React exports; refactors the wagmi-adapter connector to generic connection options, persistent connected-wallet storage, improved isAuthorized/chain handling; bumps several dependencies and adds changesets. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App
participant WagmiProvider
participant Connector as InAppConnector
participant Storage as localStorage
User->>App: open page / click connect
App->>WagmiProvider: connect(parameters)
WagmiProvider->>Connector: connect(parameters)
Connector->>Storage: read connectedWalletIds, lastChainId
alt wallet already connected
Connector-->>WagmiProvider: { accounts, chainId } (early return)
else new connection
Connector->>Connector: link wallet, getAccount()
Connector->>Storage: update connectedWalletIds (+id)
Connector->>Storage: set activeWalletId
Connector-->>WagmiProvider: { accounts, chainId }
end
WagmiProvider-->>App: connection result
sequenceDiagram
autonumber
participant App
participant Thirdweb
participant ViemAdapter
participant RPC
App->>Thirdweb: format transaction (authorizationList.address)
Thirdweb->>ViemAdapter: create WalletClient (normalized address)
ViemAdapter->>RPC: eth_sendTransaction
RPC-->>ViemAdapter: tx hash / error
ViemAdapter-->>Thirdweb: result
Thirdweb-->>App: status (pending/success/error)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
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. |
joaquim-verges
changed the title
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
size-limit report 📦
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8167 +/- ##
=======================================
Coverage 56.28% 56.29%
=======================================
Files 906 906
Lines 59208 59209 +1
Branches 4180 4176 -4
=======================================
+ Hits 33324 33330 +6
+ Misses 25779 25774 -5
Partials 105 105
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/wagmi-adapter/src/connector.ts (1)
112-179: Propagate capability-aware accounts fromconnect().The new
ConnectionOptionsgeneric promises thatconnect({ withCapabilities: true, ... })returns account objects that includecapabilities. The implementation still strips everything down to raw addresses (and even force-casts withas anyon reconnect), so callers never receive the capabilities data and the new typing lies at runtime.Handle the
withCapabilitiesflag (and existing capability data on reconnect) when building the return value.- connect: async ( - params?: ConnectionOptions<false>, - ) => { + connect: async <WithCapabilities extends boolean = false>( + params?: ConnectionOptions<WithCapabilities>, + ) => { const lastChainIdStr = await config.storage?.getItem("thirdweb:lastChainId"); const lastChainId = lastChainIdStr ? Number(lastChainIdStr) : undefined; if (params?.isReconnecting) { const { autoConnect } = await import("thirdweb/wallets"); const chainId = lastChainId || args.smartAccount?.chain?.id || 1; await autoConnect({ chain: defineChain(chainId), client, wallets: [wallet], onConnect: args.onConnect, }); const account = wallet.getAccount(); if (!account) { throw new Error("Wallet failed to reconnect"); } - return { - accounts: [getAddress(account.address)] as any, - chainId: chainId, - }; + if (account.capabilities) { + return { + accounts: [ + { + address: getAddress(account.address), + capabilities: account.capabilities, + }, + ] as const, + chainId, + }; + } + + return { + accounts: [getAddress(account.address)] as const, + chainId, + }; } const inAppOptions = params && "strategy" in params ? params : undefined; if (!inAppOptions) { throw new Error( "Missing strategy prop, pass it to connect() when connecting to this connector" ); } const chain = defineChain( inAppOptions?.chainId || lastChainId || args.smartAccount?.chain?.id || 1 ); const decoratedOptions = { ...inAppOptions, chain, client, } as InAppWalletConnectionOptions; const account = await wallet.connect(decoratedOptions); // setting up raw local storage value for autoConnect if (rawStorage) { const connectedWalletIds = rawStorage?.getItem(connectedWalletIdsKey); if (connectedWalletIds) { const connectedWalletIdsArray = JSON.parse(connectedWalletIds); if(Array.isArray(connectedWalletIdsArray)) { if(!connectedWalletIdsArray.includes(wallet.id)) { connectedWalletIdsArray.push(wallet.id); rawStorage.setItem( connectedWalletIdsKey, JSON.stringify(connectedWalletIdsArray) ); } } } else { rawStorage.setItem( connectedWalletIdsKey, JSON.stringify([wallet.id]) ); } rawStorage.setItem(activeWalletIdKey, wallet.id); } await config.storage?.setItem("thirdweb:lastChainId", chain.id); args.onConnect?.(wallet); - return { accounts: [getAddress(account.address)], chainId: chain.id }; + if (decoratedOptions.withCapabilities) { + return { + accounts: [ + { + address: getAddress(account.address), + capabilities: account.capabilities ?? {}, + }, + ] as const, + chainId: chain.id, + }; + } + + return { + accounts: [getAddress(account.address)] as const, + chainId: chain.id, + }; },
🧹 Nitpick comments (6)
apps/wagmi-demo/.env.example (1)
1-1: Add trailing newline.Add a blank line at the end of the file to comply with POSIX text file standards.
Apply this diff:
VITE_THIRDWEB_CLIENT_ID= +apps/wagmi-demo/vite.config.ts (1)
4-4: Remove redundant JSDoc type annotation.The
defineConfigimport already provides full TypeScript types, making the JSDoc annotation unnecessary.-/** @type {import('vite').UserConfig} */ export default defineConfig({apps/wagmi-demo/src/main.tsx (1)
15-15: Consider QueryClient configuration options.The QueryClient is instantiated with default settings. For production use, consider adding configuration for cache times, retry logic, and error handling.
Example configuration:
const queryClient = new QueryClient({ defaultOptions: { queries: { staleTime: 1000 * 60, // 1 minute retry: 1, }, }, });apps/wagmi-demo/package.json (1)
1-31: Consider adding bundle size tracking.As per coding guidelines, consider tracking bundle budgets via
size-limitconfiguration in package.json to ensure the demo app remains lean.Based on coding guidelines.
apps/wagmi-demo/tsconfig.base.json (2)
14-14: Clarify TODO comment about useUnknownInCatchVariables.The comment mentions this option would "require some adjustments to the codebase," but the option is already enabled (
true). Either the TODO is outdated, or the adjustments were made. Consider removing or updating the comment.- "useUnknownInCatchVariables": true, // TODO: This would normally be enabled in `strict` mode but would require some adjustments to the codebase. + "useUnknownInCatchVariables": true,
36-39: Update DOM library comment.The comment suggests DOM types are added temporarily until available via DefinitelyTyped. Modern TypeScript (5.x) includes these types by default. The comment is outdated.
"lib": [ "ES2022", // By using ES2022 we get access to the `.cause` property on `Error` instances. - "DOM" // We are adding `DOM` here to get the `fetch`, etc. types. This should be removed once these types are available via DefinitelyTyped. + "DOM" ],
📜 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 ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
.changeset/grumpy-toes-switch.md(1 hunks).changeset/quiet-gifts-lead.md(1 hunks)apps/wagmi-demo/.env.example(1 hunks)apps/wagmi-demo/.gitignore(1 hunks)apps/wagmi-demo/.npmrc(1 hunks)apps/wagmi-demo/README.md(1 hunks)apps/wagmi-demo/biome.json(1 hunks)apps/wagmi-demo/index.html(1 hunks)apps/wagmi-demo/package.json(1 hunks)apps/wagmi-demo/src/App.tsx(1 hunks)apps/wagmi-demo/src/index.css(1 hunks)apps/wagmi-demo/src/main.tsx(1 hunks)apps/wagmi-demo/src/vite-env.d.ts(1 hunks)apps/wagmi-demo/src/wagmi.ts(1 hunks)apps/wagmi-demo/tsconfig.base.json(1 hunks)apps/wagmi-demo/tsconfig.json(1 hunks)apps/wagmi-demo/vite.config.ts(1 hunks)packages/thirdweb/package.json(1 hunks)packages/thirdweb/src/adapters/viem.ts(2 hunks)packages/thirdweb/src/event/actions/get-events.ts(1 hunks)packages/thirdweb/src/exports/react.native.ts(1 hunks)packages/thirdweb/src/exports/react.ts(1 hunks)packages/thirdweb/src/transaction/actions/estimate-gas.ts(1 hunks)packages/wagmi-adapter/package.json(1 hunks)packages/wagmi-adapter/src/connector.ts(6 hunks)packages/wagmi-adapter/src/exports/thirdweb.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{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/event/actions/get-events.tspackages/thirdweb/src/transaction/actions/estimate-gas.tspackages/wagmi-adapter/src/exports/thirdweb.tspackages/thirdweb/src/exports/react.tsapps/wagmi-demo/vite.config.tsapps/wagmi-demo/src/App.tsxapps/wagmi-demo/src/main.tsxpackages/thirdweb/src/adapters/viem.tsapps/wagmi-demo/src/wagmi.tsapps/wagmi-demo/src/vite-env.d.tspackages/thirdweb/src/exports/react.native.tspackages/wagmi-adapter/src/connector.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/event/actions/get-events.tspackages/thirdweb/src/transaction/actions/estimate-gas.tspackages/wagmi-adapter/src/exports/thirdweb.tspackages/thirdweb/src/exports/react.tsapps/wagmi-demo/vite.config.tsapps/wagmi-demo/src/App.tsxapps/wagmi-demo/src/main.tsxpackages/thirdweb/src/adapters/viem.tsapps/wagmi-demo/src/wagmi.tsapps/wagmi-demo/src/vite-env.d.tspackages/thirdweb/src/exports/react.native.tspackages/wagmi-adapter/src/connector.ts
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/event/actions/get-events.tspackages/thirdweb/src/transaction/actions/estimate-gas.tspackages/thirdweb/src/exports/react.tspackages/thirdweb/src/adapters/viem.tspackages/thirdweb/src/exports/react.native.ts
packages/wagmi-adapter/**
📄 CodeRabbit inference engine (CLAUDE.md)
Wagmi ecosystem integration is in
packages/wagmi-adapter/
Files:
packages/wagmi-adapter/src/exports/thirdweb.tspackages/wagmi-adapter/package.jsonpackages/wagmi-adapter/src/connector.ts
packages/thirdweb/src/exports/**
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/exports/**: Export everything viaexports/directory, grouped by feature in the SDK public API
Every public symbol must have comprehensive TSDoc with at least one@exampleblock that compiles and custom annotation tags (@beta,@internal,@experimental)
Files:
packages/thirdweb/src/exports/react.tspackages/thirdweb/src/exports/react.native.ts
.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/grumpy-toes-switch.md.changeset/quiet-gifts-lead.md
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Track bundle budgets via
package.json#size-limit
Files:
apps/wagmi-demo/package.jsonpackages/thirdweb/package.jsonpackages/wagmi-adapter/package.json
🧠 Learnings (7)
📚 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:
apps/wagmi-demo/src/main.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 apps/{dashboard,playground-web}/**/*.{ts,tsx} : Wrap client-side data fetching calls in React Query (`tanstack/react-query`)
Applied to files:
apps/wagmi-demo/src/main.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/** : Unified `Wallet` and `Account` interfaces in wallet architecture
Applied to files:
packages/thirdweb/src/adapters/viem.ts
📚 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/wagmi-adapter/** : Wagmi ecosystem integration is in `packages/wagmi-adapter/`
Applied to files:
apps/wagmi-demo/README.mdpackages/wagmi-adapter/package.json
📚 Learning: 2025-08-29T15:37:38.513Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: AGENTS.md:0-0
Timestamp: 2025-08-29T15:37:38.513Z
Learning: Applies to biome.json : Biome governs formatting and linting; project rules live in biome.json
Applied to files:
apps/wagmi-demo/biome.json
📚 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 biome.json : Biome is the primary linter/formatter; rules are defined in `biome.json`
Applied to files:
apps/wagmi-demo/biome.json
📚 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 src/exports/react.native.ts : React Native specific exports are in `src/exports/react.native.ts`
Applied to files:
packages/thirdweb/src/exports/react.native.ts
🧬 Code graph analysis (5)
packages/thirdweb/src/event/actions/get-events.ts (1)
packages/thirdweb/src/exports/thirdweb.ts (1)
Hex(230-230)
apps/wagmi-demo/src/App.tsx (3)
packages/thirdweb/src/exports/react.native.ts (4)
useConnect(79-79)useDisconnect(81-81)useSendTransaction(104-104)useConnectedWallets(80-80)packages/thirdweb/src/exports/react.ts (4)
useConnect(89-89)useDisconnect(91-91)useSendTransaction(129-129)useConnectedWallets(90-90)apps/wagmi-demo/src/wagmi.ts (1)
chain(16-16)
apps/wagmi-demo/src/main.tsx (3)
packages/thirdweb/src/exports/react.native.ts (1)
ThirdwebProvider(109-109)packages/thirdweb/src/exports/react.ts (1)
ThirdwebProvider(134-134)apps/wagmi-demo/src/wagmi.ts (1)
config(19-33)
apps/wagmi-demo/src/wagmi.ts (2)
packages/thirdweb/test/src/test-clients.js (1)
clientId(3-3)packages/wagmi-adapter/src/connector.ts (1)
inAppWalletConnector(100-251)
packages/wagmi-adapter/src/connector.ts (2)
packages/wagmi-adapter/src/exports/thirdweb.ts (2)
ConnectionOptions(4-4)InAppWalletParameters(2-2)apps/wagmi-demo/src/wagmi.ts (3)
client(12-14)config(19-33)chain(16-16)
🪛 dotenv-linter (3.3.0)
apps/wagmi-demo/.env.example
[warning] 1-1: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
⏰ 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: Size
- GitHub Check: Unit Tests
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Build Packages
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (18)
packages/thirdweb/src/event/actions/get-events.ts (1)
279-279: LGTM! Type safety improvement.The explicit
as Hexcast aligns with the pattern used for other fields (blockHash, data, transactionHash) and ensures type compatibility with viem'sformatLogfunction.packages/thirdweb/src/exports/react.native.ts (1)
90-90: LGTM! Consistent with web export.The export structure and placement mirror the web export in
react.ts, correctly sharing the same core implementation. The TSDoc compliance verification requested for the web export applies here as well.Based on learnings.
.changeset/quiet-gifts-lead.md (1)
1-6: LGTM!The changeset correctly specifies a patch release for the dependency update, following the coding guidelines for version bump rules.
packages/thirdweb/src/adapters/viem.ts (2)
435-438: LGTM!Good refactor to call
wallet.getAccount()once and store it in a local variable. This improves efficiency by avoiding repeated function calls and makes the null check more explicit and readable.
460-460: LGTM!Correctly uses the cached
accountvariable and properly normalizes the address withgetAddress()for the viem client. This ensures the address is properly checksummed.apps/wagmi-demo/.npmrc (1)
1-1: Removelegacy-peer-depsfrom apps/wagmi-demo/.npmrc
No peer dependency conflicts arise when installing without this flag, so it can be safely deleted.packages/thirdweb/package.json (1)
42-42: Approve viem 2.37.9 upgrade Verified that 2.37.9 is the latest published version and no security advisories were reported.apps/wagmi-demo/src/vite-env.d.ts (1)
1-1: LGTM!Standard Vite TypeScript reference directive that enables global types for
import.meta, HMR, and other Vite client APIs.apps/wagmi-demo/.gitignore (1)
1-26: LGTM!Standard and comprehensive gitignore patterns for a Vite/React application. Correctly ignores build artifacts, logs, dependencies, local files, and environment configuration while preserving
.vscode/extensions.json.apps/wagmi-demo/biome.json (1)
1-13: Approve subfolder Biome config Subfolder config aligns with rootbiome.jsonand intentionally overrideslineWidth: 120and enables import organization; no inconsistencies detected.packages/thirdweb/src/transaction/actions/estimate-gas.ts (1)
119-126: Authorization types already useaddressfield; no inconsistencies found.apps/wagmi-demo/tsconfig.json (1)
1-10: LGTM! Clean TypeScript configuration.The configuration appropriately extends the shared base config, includes the source directory, and sets up the base URL for module resolution. The setup is minimal and follows best practices for a demo application.
apps/wagmi-demo/tsconfig.base.json (1)
1-47: Excellent strict TypeScript configuration.The comprehensive set of type-checking rules and modern ES/Node configuration provides strong safety guarantees for the codebase. The inline comments documenting each option's purpose are helpful.
apps/wagmi-demo/src/main.tsx (2)
13-13: Verify Buffer polyfill necessity.
Unable to confirm usage ofBufferin your bundled dependencies (nonode_modulesavailable in this environment); please manually ensure that wagmi/thirdweb (or any other package) truly requires the Node.js Buffer API in the browser.
20-26: Provider nesting is correct. ThirdwebProvider wrapping WagmiProvider aligns with official thirdweb documentation.packages/wagmi-adapter/src/exports/thirdweb.ts (1)
3-4: Confirm intended public API for new type exports
The typesInAppWalletConnectorandConnectionOptionsare only referenced internally inpackages/wagmi-adapter/src/connector.tsand re-exported inpackages/wagmi-adapter/src/exports/thirdweb.ts—they aren’t used in any examples or docs. If these are meant for consumers, please add documentation and usage examples; otherwise remove them from the public exports.apps/wagmi-demo/package.json (1)
28-28: Verified: TypeScript 5.8.3 availabilityTypeScript 5.8.3 is a valid stable release (April 7, 2025); the latest stable version as of October 1, 2025 is v5.9.2. (github.com)
apps/wagmi-demo/src/App.tsx (1)
5-5: Add an explicit return type forApp.Our TypeScript guidelines require explicit return types for component functions. Declaring
(): JSX.Elementkeeps the component signature consistent and avoids implicit-any regressions in future edits.-function App() { +function App(): JSX.Element {As per coding guidelines
⛔ Skipped due to learnings
Learnt from: CR PR: thirdweb-dev/js#0 File: AGENTS.md:0-0 Timestamp: 2025-08-29T15:37:38.513Z Learning: Applies to **/*.{ts,tsx} : Use explicit function declarations and explicit return types in TypeScriptLearnt from: CR PR: thirdweb-dev/js#0 File: CLAUDE.md:0-0 Timestamp: 2025-07-18T19:19:55.613Z Learning: Applies to **/*.{ts,tsx} : Write idiomatic TypeScript with explicit function declarations and return typesLearnt from: MananTank PR: thirdweb-dev/js#7977 File: apps/playground-web/src/app/page.tsx:61-65 Timestamp: 2025-09-03T23:35:50.476Z Learning: In the thirdweb-dev/js codebase, specifically for React components in **/*.{ts,tsx} files, do not suggest adding explicit return types like `: JSX.Element` or `: React.ReactElement` to function components. The project maintainer MananTank has explicitly declined these suggestions.
| "@types/react": "19.1.0", | ||
| "@types/react-dom": "19.1.0", | ||
| "@vitejs/plugin-react": "5.0.4", | ||
| "@wagmi/cli": "latest", |
There was a problem hiding this comment.
Pin @wagmi/cli to a specific version.
Using "latest" for @wagmi/cli creates non-deterministic builds. Lock files will resolve this at install time, but explicit pinning improves reproducibility and makes version updates explicit.
Replace with a specific version:
- "@wagmi/cli": "latest",
+ "@wagmi/cli": "^2.1.0",📝 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.
| "@wagmi/cli": "latest", | |
| "@wagmi/cli": "^2.1.0", |
🤖 Prompt for AI Agents
In apps/wagmi-demo/package.json around line 26, the dependency for "@wagmi/cli"
is pinned to "latest" which makes builds non-deterministic; replace the "latest"
specifier with a specific published version (an exact version string, not a
range) appropriate for the project, update the lockfile by running your package
manager (npm/yarn/pnpm) to record the resolved version, and commit both
package.json and the updated lockfile so future installs are reproducible.
| "@types/react-dom": "19.1.0", | ||
| "@vitejs/plugin-react": "5.0.4", | ||
| "@wagmi/cli": "latest", | ||
| "buffer": "^6.0.3", |
There was a problem hiding this comment.
Move buffer to dependencies.
The buffer package is imported and used at runtime in main.tsx (globalThis.Buffer assignment), but it's listed in devDependencies. This will cause runtime failures in production builds.
Apply this diff:
"dependencies": {
"@tanstack/react-query": "5.90.2",
"@thirdweb-dev/wagmi-adapter": "workspace:*",
+ "buffer": "^6.0.3",
"react": "19.1.0",
"react-dom": "19.1.0",
"thirdweb": "workspace:*",
"viem": "2.37.9",
"wagmi": "2.17.5"
},
"devDependencies": {
"@biomejs/biome": "2.0.6",
"@types/react": "19.1.0",
"@types/react-dom": "19.1.0",
"@vitejs/plugin-react": "5.0.4",
"@wagmi/cli": "latest",
- "buffer": "^6.0.3",
"typescript": "5.8.3",
"vite": "7.1.7"
}📝 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.
| "buffer": "^6.0.3", | |
| "dependencies": { | |
| "@tanstack/react-query": "5.90.2", | |
| "@thirdweb-dev/wagmi-adapter": "workspace:*", | |
| "buffer": "^6.0.3", | |
| "react": "19.1.0", | |
| "react-dom": "19.1.0", | |
| "thirdweb": "workspace:*", | |
| "viem": "2.37.9", | |
| "wagmi": "2.17.5" | |
| }, | |
| "devDependencies": { | |
| "@biomejs/biome": "2.0.6", | |
| "@types/react": "19.1.0", | |
| "@types/react-dom": "19.1.0", | |
| "@vitejs/plugin-react": "5.0.4", | |
| "@wagmi/cli": "latest", | |
| "typescript": "5.8.3", | |
| "vite": "7.1.7" | |
| } |
🤖 Prompt for AI Agents
In apps/wagmi-demo/package.json around line 27, the buffer package is currently
listed under devDependencies but is required at runtime (used in main.tsx to set
globalThis.Buffer); move "buffer": "^6.0.3" from devDependencies into
dependencies so it is installed in production builds — update package.json
accordingly (or run your package manager: npm install --save buffer or yarn add
buffer) and then rebuild to ensure the runtime import succeeds.
| "@wagmi/cli": "latest", | ||
| "buffer": "^6.0.3", | ||
| "typescript": "5.8.3", | ||
| "vite": "7.1.7" |
There was a problem hiding this comment.
🧩 Analysis chain
Verify vite version 7.1.7 availability.
The specified Vite version (7.1.7) appears to be ahead of commonly available releases. Confirm this version exists and is stable.
🌐 Web query:
What is the latest stable version of Vite as of October 2025?
💡 Result:
As of October 1, 2025, the latest stable Vite release is 7.1.4. [1][2]
Sources:
[1] npm package page for vite (shows v7.1.4).
[2] Vite blog announcing Vite 7.0 (June 24, 2025).
Replace the Vite version with the latest stable release (7.1.4)
The specified version (7.1.7) isn’t published; update apps/wagmi-demo/package.json to "vite": "7.1.4".
🤖 Prompt for AI Agents
In apps/wagmi-demo/package.json around line 29, the Vite dependency is set to an
unavailable version "7.1.7"; change it to the latest stable published release
"7.1.4" by updating the "vite" value to "7.1.4" and then run npm/yarn install to
refresh lockfile and verify the project builds.
| {account && ( | ||
| <div> | ||
| <h2>Transact</h2> | ||
| <button onClick={() => sendTransaction({ to: account.address, value: 0n })} type="button"> | ||
| Send Tx | ||
| </button> | ||
| <div>{isPending ? "Sending..." : ""}</div> | ||
| <div>{isSuccess ? `Success: ${sendTxData}` : isError ? sendTxError?.message : ""}</div> | ||
| </div> |
There was a problem hiding this comment.
Hide the transact section until the wallet is connected.
account is always a truthy object from useAccount, so this block renders even while disconnected. Clicking “Send Tx” in that state calls sendTransaction with account.address === undefined, which immediately throws a “connector not ready”/validation error. Guard the section (and the call) with the connected status and a defined address instead.
- {account && (
+ {account.status === "connected" && account.address && (
<div>
<h2>Transact</h2>
- <button onClick={() => sendTransaction({ to: account.address, value: 0n })} type="button">
+ <button onClick={() => sendTransaction({ to: account.address, value: 0n })} type="button">
Send Tx
</button>
<div>{isPending ? "Sending..." : ""}</div>
<div>{isSuccess ? `Success: ${sendTxData}` : isError ? sendTxError?.message : ""}</div>
</div>
)}📝 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.
| {account && ( | |
| <div> | |
| <h2>Transact</h2> | |
| <button onClick={() => sendTransaction({ to: account.address, value: 0n })} type="button"> | |
| Send Tx | |
| </button> | |
| <div>{isPending ? "Sending..." : ""}</div> | |
| <div>{isSuccess ? `Success: ${sendTxData}` : isError ? sendTxError?.message : ""}</div> | |
| </div> | |
| {account.status === "connected" && account.address && ( | |
| <div> | |
| <h2>Transact</h2> | |
| <button | |
| onClick={() => sendTransaction({ to: account.address, value: 0n })} | |
| type="button" | |
| > | |
| Send Tx | |
| </button> | |
| <div>{isPending ? "Sending..." : ""}</div> | |
| <div> | |
| {isSuccess | |
| ? `Success: ${sendTxData}` | |
| : isError | |
| ? sendTxError?.message | |
| : ""} | |
| </div> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
In apps/wagmi-demo/src/App.tsx around lines 56 to 64, the Transact UI is
rendered and the Send Tx handler is invoked even when the wallet is not
connected because useAccount returns a truthy object without an address; update
the rendering and click handler to require account.isConnected (or connected
boolean) and a defined account.address before showing the section and before
calling sendTransaction, and also disable or no-op the button when address is
undefined to prevent calling sendTransaction with an undefined recipient.
| "allowSyntheticDefaultImports": true, | ||
| "forceConsistentCasingInFileNames": true, | ||
| "verbatimModuleSyntax": true, | ||
| "importHelpers": true, // This is only used for build validation. Since we do not have `tslib` installed, this will fail if we accidentally make use of anything that'd require injection of helpers. |
There was a problem hiding this comment.
Reconsider importHelpers configuration.
Setting importHelpers: true without installing tslib will cause build failures if TypeScript needs to inject helper functions (for async/await, spread, etc.). The comment indicates this is intentional for build validation, but this creates a fragile setup for a demo app where code changes might unexpectedly require helpers.
Consider one of these approaches:
- Set
importHelpers: false(recommended for demo) - Add
tslibto dependencies if helper injection is acceptable
- "importHelpers": true, // This is only used for build validation. Since we do not have `tslib` installed, this will fail if we accidentally make use of anything that'd require injection of helpers.
+ "importHelpers": false,📝 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.
| "importHelpers": true, // This is only used for build validation. Since we do not have `tslib` installed, this will fail if we accidentally make use of anything that'd require injection of helpers. | |
| "importHelpers": false, |
🤖 Prompt for AI Agents
In apps/wagmi-demo/tsconfig.base.json around line 30, importHelpers is set to
true but tslib is not installed which will cause fragile build failures if
TypeScript emits helper injections; either set "importHelpers" to false in this
tsconfig for the demo to avoid requiring tslib, or install and add "tslib" to
the project dependencies (package.json) if you want to keep importHelpers=true
so emitted helpers are imported instead of inlined.
| import { defineConfig } from "vite"; | ||
|
|
||
| // https://vitejs.dev/config/ | ||
| /** @type {import('vite').UserConfig} */ | ||
| export default defineConfig({ | ||
| server: { | ||
| port: 3000, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Missing React plugin for JSX/TSX transformation.
The wagmi-demo app uses React (see src/App.tsx and src/main.tsx), but this Vite config doesn't include @vitejs/plugin-react. Without it, JSX/TSX files won't be transformed and the build will fail.
Add the React plugin:
import { defineConfig } from "vite";
+import react from "@vitejs/plugin-react";
// https://vitejs.dev/config/
-/** @type {import('vite').UserConfig} */
export default defineConfig({
+ plugins: [react()],
server: {
port: 3000,
},
});Note: Ensure @vitejs/plugin-react is listed in package.json dependencies.
📝 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.
| import { defineConfig } from "vite"; | |
| // https://vitejs.dev/config/ | |
| /** @type {import('vite').UserConfig} */ | |
| export default defineConfig({ | |
| server: { | |
| port: 3000, | |
| }, | |
| }); | |
| import { defineConfig } from "vite"; | |
| import react from "@vitejs/plugin-react"; | |
| // https://vitejs.dev/config/ | |
| export default defineConfig({ | |
| plugins: [react()], | |
| server: { | |
| port: 3000, | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In apps/wagmi-demo/vite.config.ts around lines 1 to 9, the Vite config is
missing the @vitejs/plugin-react plugin so JSX/TSX won't be transformed; fix it
by importing the plugin from '@vitejs/plugin-react' at the top, add it to the
defineConfig({ plugins: [react()] }) array, and ensure @vitejs/plugin-react is
declared in package.json dependencies (run npm/yarn add if missing).
| export { useSwitchActiveWalletChain } from "../react/core/hooks/wallets/useSwitchActiveWalletChain.js"; | ||
| export { useWaitForCallsReceipt } from "../react/core/hooks/wallets/useWaitForCallsReceipt.js"; | ||
| export { useConnectionManager } from "../react/core/providers/connection-manager.js"; | ||
| export { useAddConnectedWallet } from "../react/core/hooks/wallets/useAddConnectedWallet.js"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify TSDoc compliance for the new hook.
The export structure and placement look good. However, ensure that the implementation of useAddConnectedWallet in ../react/core/hooks/wallets/useAddConnectedWallet.js includes comprehensive TSDoc with at least one compiling @example block and a custom annotation tag (@beta, @internal, @experimental, etc.), as required by the coding guidelines for public exports.
As per coding guidelines.
🏁 Script executed:
#!/bin/bash
# Verify TSDoc presence in useAddConnectedWallet implementation
# Check if the hook has TSDoc with @example and custom tags
fd -t f "useAddConnectedWallet.js" packages/thirdweb/src/react/core/hooks/wallets/ --exec cat {} | grep -A 30 "export.*useAddConnectedWallet"Length of output: 0
Running a search for the actual implementation file and inspecting its header for TSDoc compliance:
🏁 Script executed:
#!/bin/bash
# Locate implementation of useAddConnectedWallet and print first 50 lines
rg -l "useAddConnectedWallet" -g "packages/thirdweb/src/react/core/hooks/wallets/*" \
| xargs -I{} sed -n '1,50p' {}Length of output: 6501
Add a recognized stability tag to the hook’s TSDoc
The implementation’s JSDoc already has an @example and a @walletConnection tag, but all public exports must also include a stability annotation (@beta, @internal, or @experimental). Add one of these to useAddConnectedWallet.
🤖 Prompt for AI Agents
In packages/thirdweb/src/exports/react.ts line 100, the export for
useAddConnectedWallet lacks the required stability TSDoc on the hook itself;
open the hook implementation at
packages/thirdweb/src/exports/react/core/hooks/wallets/useAddConnectedWallet.js
(or its .ts source), locate the hook's JSDoc block and add a stability
annotation such as @beta (or @experimental/@internal as appropriate) to the
existing comment block (keeping the current @example and @walletConnection
tags), then save—no changes to the export line are needed.
17e93dd to
3fed520
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/thirdweb/package.json (2)
325-355: Define missing size-limit budgets in packages/thirdweb/package.json
Add asize-limitsection with your current bundle size budgets and runpnpm sizeto ensure it still passes after Vite/viem upgrades.
70-97: Upgrade @vitejs/plugin-react to a Vite 7–compatible release
@vitejs/plugin-react@5.0.4 peers with vite@^4; bump to the next major (>=6.0.0) which lists vite@^7 in its peerDependencies.packages/wagmi-adapter/src/connector.ts (1)
114-138: Fix type safety in the connect function signature and reconnect path.The implementation has type safety issues:
- Line 114: The function parameter is typed as
ConnectionOptions<false>but should be genericConnectionOptions<withCapabilities>to match the Properties interface definition (lines 37-47).- Line 135: The
as anycast bypasses TypeScript's type checking and can cause runtime errors if the return type doesn't match expectations.- The reconnect path doesn't handle the
withCapabilitiesparameter, so it always returns simple address arrays even when capabilities are requested.Apply this diff to fix the type safety:
- connect: async (params?: ConnectionOptions<false>) => { + connect: async <withCapabilities extends boolean = false>( + params?: ConnectionOptions<withCapabilities>, + ) => { const lastChainIdStr = await config.storage?.getItem( "thirdweb:lastChainId", ); const lastChainId = lastChainIdStr ? Number(lastChainIdStr) : undefined; if (params?.isReconnecting) { const { autoConnect } = await import("thirdweb/wallets"); const chainId = lastChainId || args.smartAccount?.chain?.id || 1; await autoConnect({ chain: defineChain(chainId), client, wallets: [wallet], onConnect: args.onConnect, }); const account = wallet.getAccount(); if (!account) { throw new Error("Wallet failed to reconnect"); } return { - accounts: [getAddress(account.address)] as any, + accounts: [getAddress(account.address)] as withCapabilities extends true + ? readonly { address: `0x${string}`; capabilities: Record<string, unknown> }[] + : readonly `0x${string}`[], chainId: chainId, }; }
♻️ Duplicate comments (3)
apps/wagmi-demo/tsconfig.base.json (1)
30-30: importHelpers without tslib will break builds; flip it off or add tslib.Same concern as earlier review. Keep demo robust.
- "importHelpers": true, // This is only used for build validation. Since we do not have `tslib` installed, this will fail if we accidentally make use of anything that'd require injection of helpers. + "importHelpers": false,#!/bin/bash set -euo pipefail # 1) Check if apps/wagmi-demo declares tslib if [ -f "apps/wagmi-demo/package.json" ]; then echo "apps/wagmi-demo/package.json tslib:" jq -r '(.dependencies.tslib // .devDependencies.tslib // "MISSING")' apps/wagmi-demo/package.json else echo "apps/wagmi-demo/package.json not found; searching workspace for tslib…" fd -t f package.json | xargs -I {} sh -c 'echo "{}: $(jq -r \".dependencies.tslib // .devDependencies.tslib // empty\" {})"' | rg -n '.+' fi # 2) Verify if tsc is used for build (tsconfig.build.json) and whether include/noEmit are set echo "tsconfig files extending tsconfig.base.json under apps/wagmi-demo:" rg -n --glob 'apps/wagmi-demo/**/tsconfig*.json' -C1 '"extends"\s*:\s*".*tsconfig\.base\.json"' echo "Show include fields:" fd -t f 'tsconfig*.json' apps/wagmi-demo | xargs -I {} sh -c 'echo "--- {}"; cat "{}" | jq ".include? // []"'apps/wagmi-demo/vite.config.ts (1)
1-9: Missing React plugin for JSX/TSX transformation.This issue was already flagged in a previous review: the config lacks
@vitejs/plugin-react, so JSX/TSX files won't transform and the build will fail.packages/thirdweb/src/exports/react.ts (1)
85-85: Add stability annotation to useAddConnectedWallet JSDoc
The JSDoc in packages/thirdweb/src/react/core/hooks/wallets/useAddConnectedWallet.ts is missing a required@beta,@internal, or@experimentaltag.
🧹 Nitpick comments (9)
apps/wagmi-demo/tsconfig.base.json (3)
33-35: Use bundler-friendly module settings for Vite.Prefer Bundler/ESNext to avoid NodeNext ESM path headaches.
- "moduleResolution": "NodeNext", - "module": "NodeNext", + "moduleResolution": "Bundler", + "module": "ESNext",
35-39: Target a more modern JS version.Node 16 is EOL; Vite 5 targets Node 18+. Align target with ES2022.
- "target": "ES2021", // Setting this to `ES2021` enables native support for `Node v16+`: https://github.com/microsoft/TypeScript/wiki/Node-Target-Mapping. + "target": "ES2022", - "ES2022", // By using ES2022 we get access to the `.cause` property on `Error` instances. + "ES2022", // Keep ES2022 lib; `.cause` remains available in types.
14-14: Comment/code mismatch for useUnknownInCatchVariables.It’s enabled. Either keep it and fix usages or turn it off now.
Option A (keep strict):
- Audit catch blocks and narrow unknown before use.
Option B (defer strictness):
- "useUnknownInCatchVariables": true, // TODO: This would normally be enabled in `strict` mode but would require some adjustments to the codebase. + "useUnknownInCatchVariables": false, // Deferred; enable after auditing catch blocks.apps/wagmi-demo/vite.config.ts (1)
4-4: Remove redundant JSDoc comment.The
@typeannotation is unnecessary becausedefineConfigalready provides full type safety. Removing it reduces clutter.Apply this diff:
-/** @type {import('vite').UserConfig} */ export default defineConfig({apps/wagmi-demo/.env.example (1)
1-1: Add trailing newline for consistency.The file is missing a blank line at the end, which many editors and linters expect by convention.
Apply this diff:
VITE_THIRDWEB_CLIENT_ID= +apps/wagmi-demo/src/main.tsx (1)
15-15: Consider configuring QueryClient for production.The QueryClient uses default settings, which may not be optimal. Consider adding configuration for retry logic, stale time, and garbage collection.
Example configuration:
-const queryClient = new QueryClient(); +const queryClient = new QueryClient({ + defaultOptions: { + queries: { + staleTime: 1000 * 60, // 1 minute + gcTime: 1000 * 60 * 5, // 5 minutes + retry: 1, + }, + }, +});.changeset/quiet-gifts-lead.md (1)
5-5: Consider a more descriptive summaryOptional: clarify which deps changed (e.g., viem, vite, @vitejs/plugin-react) or note “dev deps only / no public API changes” to help consumers scanning release notes.
packages/wagmi-adapter/package.json (1)
45-54: Add bundle size budget (size-limit) for this packageTrack bundle size in CI per guidelines. Add a size script and size-limit config.
Apply this diff (adjust limit after measuring):
"scripts": { "build": "pnpm clean && pnpm build:cjs && pnpm build:esm && pnpm build:types", "build:cjs": "tsc --project ./tsconfig.build.json --module commonjs --outDir ./dist/cjs --verbatimModuleSyntax false && printf '{\"type\":\"commonjs\"}' > ./dist/cjs/package.json", "build:esm": "tsc --project ./tsconfig.build.json --module es2020 --outDir ./dist/esm && printf '{\"type\": \"module\",\"sideEffects\":false}' > ./dist/esm/package.json", "build:types": "tsc --project ./tsconfig.build.json --module esnext --declarationDir ./dist/types --emitDeclarationOnly --declaration --declarationMap", "clean": "rimraf dist", "fix": "biome check ./src --fix", "format": "biome format ./src --write", - "lint": "biome check ./src" + "lint": "biome check ./src", + "size": "size-limit" }, + "size-limit": [ + { "path": "dist/esm/exports/thirdweb.js", "limit": "6 kB" } + ]If size-limit isn’t installed at repo root, add it to devDependencies here.
packages/wagmi-adapter/src/connector.ts (1)
53-54: Consider storage consistency implications.The connector uses two storage mechanisms:
- Wagmi's
config.storageforlastChainId- Direct
localStorageaccess for wallet IDsThis dual approach could lead to inconsistent state if one storage is cleared independently (e.g., user clears localStorage but not IndexedDB if wagmi uses it).
Consider either:
- Documenting this behavior and its implications for users
- Using wagmi's storage abstraction for wallet IDs as well for consistency
- Adding a cleanup/sync mechanism to detect and handle storage inconsistencies
Example using wagmi storage:
// In connect function await config.storage?.setItem(connectedWalletIdsKey, JSON.stringify(connectedWalletIdsArray)); await config.storage?.setItem(activeWalletIdKey, wallet.id); // In isAuthorized const connectedWalletIds = await config.storage?.getItem(connectedWalletIdsKey);Also applies to: 109-112
📜 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 ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
.changeset/grumpy-toes-switch.md(1 hunks).changeset/quiet-gifts-lead.md(1 hunks)apps/wagmi-demo/.env.example(1 hunks)apps/wagmi-demo/.gitignore(1 hunks)apps/wagmi-demo/.npmrc(1 hunks)apps/wagmi-demo/README.md(1 hunks)apps/wagmi-demo/biome.json(1 hunks)apps/wagmi-demo/index.html(1 hunks)apps/wagmi-demo/package.json(1 hunks)apps/wagmi-demo/src/App.tsx(1 hunks)apps/wagmi-demo/src/index.css(1 hunks)apps/wagmi-demo/src/main.tsx(1 hunks)apps/wagmi-demo/src/vite-env.d.ts(1 hunks)apps/wagmi-demo/src/wagmi.ts(1 hunks)apps/wagmi-demo/tsconfig.base.json(1 hunks)apps/wagmi-demo/tsconfig.json(1 hunks)apps/wagmi-demo/vite.config.ts(1 hunks)packages/thirdweb/package.json(3 hunks)packages/thirdweb/src/adapters/viem.ts(2 hunks)packages/thirdweb/src/event/actions/get-events.ts(1 hunks)packages/thirdweb/src/exports/react.native.ts(1 hunks)packages/thirdweb/src/exports/react.ts(1 hunks)packages/thirdweb/src/transaction/actions/estimate-gas.ts(1 hunks)packages/wagmi-adapter/package.json(1 hunks)packages/wagmi-adapter/src/connector.ts(6 hunks)packages/wagmi-adapter/src/exports/thirdweb.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- apps/wagmi-demo/.gitignore
- apps/wagmi-demo/index.html
- apps/wagmi-demo/.npmrc
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/thirdweb/src/transaction/actions/estimate-gas.ts
- packages/wagmi-adapter/src/exports/thirdweb.ts
- apps/wagmi-demo/src/App.tsx
- packages/thirdweb/src/adapters/viem.ts
- apps/wagmi-demo/tsconfig.json
- apps/wagmi-demo/package.json
- packages/thirdweb/src/exports/react.native.ts
- .changeset/grumpy-toes-switch.md
- apps/wagmi-demo/src/vite-env.d.ts
- apps/wagmi-demo/src/wagmi.ts
🧰 Additional context used
📓 Path-based instructions (7)
.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/quiet-gifts-lead.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:
apps/wagmi-demo/vite.config.tspackages/wagmi-adapter/src/connector.tspackages/thirdweb/src/exports/react.tsapps/wagmi-demo/src/main.tsxpackages/thirdweb/src/event/actions/get-events.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
apps/wagmi-demo/vite.config.tspackages/wagmi-adapter/src/connector.tspackages/thirdweb/src/exports/react.tsapps/wagmi-demo/src/main.tsxpackages/thirdweb/src/event/actions/get-events.ts
packages/wagmi-adapter/**
📄 CodeRabbit inference engine (CLAUDE.md)
Wagmi ecosystem integration is in
packages/wagmi-adapter/
Files:
packages/wagmi-adapter/src/connector.tspackages/wagmi-adapter/package.json
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Track bundle budgets via
package.json#size-limit
Files:
packages/wagmi-adapter/package.jsonpackages/thirdweb/package.json
packages/thirdweb/src/exports/**
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/exports/**: Export everything viaexports/directory, grouped by feature in the SDK public API
Every public symbol must have comprehensive TSDoc with at least one@exampleblock that compiles and custom annotation tags (@beta,@internal,@experimental)
Files:
packages/thirdweb/src/exports/react.ts
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/exports/react.tspackages/thirdweb/src/event/actions/get-events.ts
🧠 Learnings (5)
📚 Learning: 2025-08-29T15:37:38.513Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: AGENTS.md:0-0
Timestamp: 2025-08-29T15:37:38.513Z
Learning: Applies to biome.json : Biome governs formatting and linting; project rules live in biome.json
Applied to files:
apps/wagmi-demo/biome.json
📚 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 biome.json : Biome is the primary linter/formatter; rules are defined in `biome.json`
Applied to files:
apps/wagmi-demo/biome.json
📚 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/wagmi-adapter/** : Wagmi ecosystem integration is in `packages/wagmi-adapter/`
Applied to files:
packages/wagmi-adapter/package.jsonapps/wagmi-demo/README.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 apps/{dashboard,playground-web}/**/*.{ts,tsx} : Wrap client-side data fetching calls in React Query (`tanstack/react-query`)
Applied to files:
apps/wagmi-demo/src/main.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:
apps/wagmi-demo/src/main.tsx
🧬 Code graph analysis (3)
packages/wagmi-adapter/src/connector.ts (2)
packages/wagmi-adapter/src/exports/thirdweb.ts (1)
ConnectionOptions(2-2)apps/wagmi-demo/src/wagmi.ts (2)
config(19-33)chain(16-16)
apps/wagmi-demo/src/main.tsx (3)
packages/thirdweb/src/exports/react.native.ts (1)
ThirdwebProvider(109-109)packages/thirdweb/src/exports/react.ts (1)
ThirdwebProvider(134-134)apps/wagmi-demo/src/wagmi.ts (1)
config(19-33)
packages/thirdweb/src/event/actions/get-events.ts (1)
packages/thirdweb/src/exports/thirdweb.ts (1)
Hex(230-230)
🪛 dotenv-linter (3.3.0)
apps/wagmi-demo/.env.example
[warning] 1-1: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
⏰ 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, webpack)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Unit Tests
- GitHub Check: Build Packages
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (11)
packages/thirdweb/src/event/actions/get-events.ts (1)
279-279: LGTM! Improved type consistency.The explicit
as Hexcast aligns with the existing pattern used for other Hex fields (blockHash,data,transactionHash) in the same transformation, making the type handling consistent and self-documenting.apps/wagmi-demo/tsconfig.base.json (1)
3-3: Base config has empty include; ensure child tsconfigs define includes.Avoid accidentally type-checking nothing.
If missing, set includes in apps/wagmi-demo/tsconfig.json and tsconfig.build.json (e.g., ["src", "vite-env.d.ts"]).
apps/wagmi-demo/biome.json (1)
1-13: LGTM!The Biome configuration appropriately enables formatting with 120-character line width, linting, and import organization. This aligns with project conventions.
Based on learnings
apps/wagmi-demo/src/index.css (1)
1-21: LGTM!The theme styles correctly implement dark mode defaults with a light mode override via
prefers-color-scheme. Font rendering optimizations and color tokens are well-defined.apps/wagmi-demo/README.md (1)
1-37: LGTM!The README accurately documents the Wagmi demo's purpose, configuration, and usage. The code snippet matches the actual implementation in
src/wagmi.ts, and setup instructions are clear.apps/wagmi-demo/src/main.tsx (2)
1-13: LGTM!The Buffer polyfill on line 13 is necessary for browser compatibility with packages that depend on Node.js built-ins. The imports are correctly organized.
18-27: LGTM!The provider nesting is correctly ordered: ThirdwebProvider wraps WagmiProvider, which wraps QueryClientProvider. This ensures each provider has access to the contexts it depends on. The non-null assertion for the root element is safe.
packages/wagmi-adapter/package.json (1)
8-8: DevDep bump looks fine@wagmi/core 2.21.2 is within the peer range (^2.16.0). No action needed.
packages/thirdweb/package.json (1)
42-42: Viem bump — re-run typechecks and affected tests2.37.9 includes API/typing tweaks across 2.x. Please re-run typecheck and critical flows (wallet client, gas estimation) to catch subtle breaks.
Run repo checks:
packages/wagmi-adapter/src/connector.ts (2)
109-112: LGTM!The conditional localStorage initialization correctly handles SSR and Node.js environments where
windowis unavailable.
206-212: LGTM!The lastChainId storage handling and chain selection logic correctly handles the persistence and fallback chain selection with appropriate precedence.
| "thirdweb": patch | ||
| --- | ||
|
|
||
| Update dependencies |
There was a problem hiding this comment.
🧩 Analysis chain
Confirm bump type (patch vs minor) and coverage of all affected packages
Per guidelines: patch for non‑API changes; minor for new/modified public API. PR notes mention new public exports (e.g., useAddConnectedWallet, ConnectionOptions, InAppWalletConnector) potentially re‑exported from the "thirdweb" package. If true, this changeset should be minor, and all packages with public API additions need their own changeset entries. As per coding guidelines.
Run to validate current exports and changeset entries:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check for possibly new public exports in packages/thirdweb =="
rg -n --type=ts -C2 -g 'packages/**' -P '\bexport\s+(?:\{[^}]*\b(useAddConnectedWallet|ConnectionOptions|InAppWalletConnector)\b[^}]*\}|(const|function|type|interface)\s+(useAddConnectedWallet|ConnectionOptions|InAppWalletConnector))' || true
echo
echo "== List changeset entries referencing 'thirdweb' package and their bump type =="
rg -n -C2 -g '.changeset/*.md' -P '^"thirdweb"\s*:\s*(patch|minor|major)|^---$|^[^-].*$' || true
echo
echo "== Full list of changesets (package -> bump) =="
rg -n -g '.changeset/*.md' -P '^"[^"]+"\s*:\s*(patch|minor|major)' || true
echo
echo "If 'thirdweb' re-exports any of the above symbols, ensure its changeset uses 'minor'. Also verify any other packages with new exports have corresponding changesets."Length of output: 3063
🏁 Script executed:
#!/bin/bash
set -euo pipefail
pkg=$(grep -m1 '"name"' packages/wagmi-adapter/package.json | sed 's/.*"name":[[:space:]]*"\([^"]*\)".*/\1/')
echo "wagmi-adapter package name: $pkg"
echo
echo "== Changeset entries for thirdweb and $pkg =="
rg --hidden -n -g '.changeset/*.md' -P "^\"(thirdweb|$pkg)\"\s*:\s*(patch|minor|major)" || true
echo
echo "== All changeset entries (including hidden) =="
rg --hidden -n -g '.changeset/*.md' -P '^"[^"]+"\s*:\s*(patch|minor|major)' || trueLength of output: 419
Use minor bumps for new public exports
- .changeset/quiet-gifts-lead.md: change bump from patch to minor
- .changeset/grumpy-toes-switch.md: change bump from patch to minor
🤖 Prompt for AI Agents
In .changeset/quiet-gifts-lead.md around lines 2 to 5 the changeset currently
uses a "patch" bump for "thirdweb" but the reviewer requests a minor bump for
new public exports; update the bump from "patch" to "minor" in this file and
make the equivalent change in .changeset/grumpy-toes-switch.md so both
changesets use "minor" instead of "patch".
| export type ConnectionOptions<withCapabilities extends boolean = false> = | ||
| | (MultiStepAuthArgsType & { | ||
| chainId?: number | undefined; | ||
| isReconnecting?: boolean | undefined; | ||
| withCapabilities?: withCapabilities | boolean | undefined; | ||
| }) | ||
| | (SingleStepAuthArgsType & { | ||
| chainId?: number | undefined; | ||
| isReconnecting?: boolean | undefined; | ||
| withCapabilities?: withCapabilities | boolean | undefined; | ||
| }); |
There was a problem hiding this comment.
Fix the withCapabilities field type to maintain type safety.
The field type withCapabilities?: withCapabilities | boolean | undefined includes a redundant | boolean that defeats the purpose of the generic parameter. When withCapabilities is false, this becomes false | boolean | undefined, which simplifies to boolean | undefined, losing type information.
Apply this diff to fix the type:
export type ConnectionOptions<withCapabilities extends boolean = false> =
| (MultiStepAuthArgsType & {
chainId?: number | undefined;
isReconnecting?: boolean | undefined;
- withCapabilities?: withCapabilities | boolean | undefined;
+ withCapabilities?: withCapabilities;
})
| (SingleStepAuthArgsType & {
chainId?: number | undefined;
isReconnecting?: boolean | undefined;
- withCapabilities?: withCapabilities | boolean | undefined;
+ withCapabilities?: withCapabilities;
});📝 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.
| export type ConnectionOptions<withCapabilities extends boolean = false> = | |
| | (MultiStepAuthArgsType & { | |
| chainId?: number | undefined; | |
| isReconnecting?: boolean | undefined; | |
| withCapabilities?: withCapabilities | boolean | undefined; | |
| }) | |
| | (SingleStepAuthArgsType & { | |
| chainId?: number | undefined; | |
| isReconnecting?: boolean | undefined; | |
| withCapabilities?: withCapabilities | boolean | undefined; | |
| }); | |
| export type ConnectionOptions<withCapabilities extends boolean = false> = | |
| | (MultiStepAuthArgsType & { | |
| chainId?: number | undefined; | |
| isReconnecting?: boolean | undefined; | |
| withCapabilities?: withCapabilities; | |
| }) | |
| | (SingleStepAuthArgsType & { | |
| chainId?: number | undefined; | |
| isReconnecting?: boolean | undefined; | |
| withCapabilities?: withCapabilities; | |
| }); |
🤖 Prompt for AI Agents
In packages/wagmi-adapter/src/connector.ts around lines 23 to 33, the
withCapabilities field currently uses the union type "withCapabilities?:
withCapabilities | boolean | undefined" which widens the type and loses the
generic's specificity; remove the redundant "| boolean" so the field is declared
as "withCapabilities?: withCapabilities | undefined" in both union branches to
preserve the generic-propagated literal type while still allowing undefined.
| const alreadyConnectedAccount = wallet.getAccount(); | ||
| if (alreadyConnectedAccount) { | ||
| return { | ||
| accounts: [getAddress(alreadyConnectedAccount.address)], | ||
| chainId: chain.id, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Handle the withCapabilities parameter in the early return path.
The early return for already-connected accounts doesn't respect the withCapabilities parameter. When capabilities are requested, this path returns plain addresses instead of enriched account objects.
Apply this diff to handle capabilities correctly:
const alreadyConnectedAccount = wallet.getAccount();
if (alreadyConnectedAccount) {
return {
- accounts: [getAddress(alreadyConnectedAccount.address)],
+ accounts: (params?.withCapabilities
+ ? [{ address: getAddress(alreadyConnectedAccount.address), capabilities: {} }]
+ : [getAddress(alreadyConnectedAccount.address)]) as withCapabilities extends true
+ ? readonly { address: `0x${string}`; capabilities: Record<string, unknown> }[]
+ : readonly `0x${string}`[],
chainId: chain.id,
};
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/wagmi-adapter/src/connector.ts around lines 157 to 163, the
early-return for an already-connected account ignores the withCapabilities flag
and always returns plain address strings; update this branch to check
withCapabilities and, when true, return the enriched account object(s) (e.g.,
include the normalized address via getAddress plus the capabilities payload or
structure the rest of the account fields as the rest of the code expects),
otherwise keep the existing plain-address array; ensure the returned shape
matches the function's declared types and include the chainId as before.
| if (rawStorage) { | ||
| const connectedWalletIds = rawStorage?.getItem(connectedWalletIdsKey); | ||
| if (connectedWalletIds) { | ||
| const connectedWalletIdsArray = JSON.parse(connectedWalletIds); | ||
| if (Array.isArray(connectedWalletIdsArray)) { | ||
| if (!connectedWalletIdsArray.includes(wallet.id)) { | ||
| connectedWalletIdsArray.push(wallet.id); | ||
| rawStorage.setItem( | ||
| connectedWalletIdsKey, | ||
| JSON.stringify(connectedWalletIdsArray), | ||
| ); | ||
| } | ||
| } | ||
| } else { | ||
| rawStorage.setItem( | ||
| connectedWalletIdsKey, | ||
| JSON.stringify([wallet.id]), | ||
| ); | ||
| } | ||
| rawStorage.setItem(activeWalletIdKey, wallet.id); | ||
| } |
There was a problem hiding this comment.
Add error handling for JSON parsing and simplify the storage logic.
The current implementation has several issues:
JSON.parseat lines 170 and 233 can throw on corrupted data, causing the entire connection flow to fail.- The read-modify-write pattern (lines 168-179) is susceptible to race conditions if multiple connection attempts occur concurrently.
- The nested conditionals make the code harder to follow.
Apply this diff to add error handling:
if (rawStorage) {
- const connectedWalletIds = rawStorage?.getItem(connectedWalletIdsKey);
- if (connectedWalletIds) {
- const connectedWalletIdsArray = JSON.parse(connectedWalletIds);
- if (Array.isArray(connectedWalletIdsArray)) {
- if (!connectedWalletIdsArray.includes(wallet.id)) {
- connectedWalletIdsArray.push(wallet.id);
- rawStorage.setItem(
- connectedWalletIdsKey,
- JSON.stringify(connectedWalletIdsArray),
- );
- }
- }
- } else {
- rawStorage.setItem(
- connectedWalletIdsKey,
- JSON.stringify([wallet.id]),
- );
+ try {
+ const connectedWalletIds = rawStorage.getItem(connectedWalletIdsKey);
+ let connectedWalletIdsArray: string[] = [];
+
+ if (connectedWalletIds) {
+ const parsed = JSON.parse(connectedWalletIds);
+ connectedWalletIdsArray = Array.isArray(parsed) ? parsed : [];
+ }
+
+ if (!connectedWalletIdsArray.includes(wallet.id)) {
+ connectedWalletIdsArray.push(wallet.id);
+ rawStorage.setItem(
+ connectedWalletIdsKey,
+ JSON.stringify(connectedWalletIdsArray),
+ );
+ }
+
+ rawStorage.setItem(activeWalletIdKey, wallet.id);
+ } catch (error) {
+ // Reset on parse error
+ rawStorage.setItem(
+ connectedWalletIdsKey,
+ JSON.stringify([wallet.id]),
+ );
+ rawStorage.setItem(activeWalletIdKey, wallet.id);
}
- rawStorage.setItem(activeWalletIdKey, wallet.id);
}📝 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.
| if (rawStorage) { | |
| const connectedWalletIds = rawStorage?.getItem(connectedWalletIdsKey); | |
| if (connectedWalletIds) { | |
| const connectedWalletIdsArray = JSON.parse(connectedWalletIds); | |
| if (Array.isArray(connectedWalletIdsArray)) { | |
| if (!connectedWalletIdsArray.includes(wallet.id)) { | |
| connectedWalletIdsArray.push(wallet.id); | |
| rawStorage.setItem( | |
| connectedWalletIdsKey, | |
| JSON.stringify(connectedWalletIdsArray), | |
| ); | |
| } | |
| } | |
| } else { | |
| rawStorage.setItem( | |
| connectedWalletIdsKey, | |
| JSON.stringify([wallet.id]), | |
| ); | |
| } | |
| rawStorage.setItem(activeWalletIdKey, wallet.id); | |
| } | |
| if (rawStorage) { | |
| try { | |
| const connectedWalletIds = rawStorage.getItem(connectedWalletIdsKey); | |
| let connectedWalletIdsArray: string[] = []; | |
| if (connectedWalletIds) { | |
| const parsed = JSON.parse(connectedWalletIds); | |
| connectedWalletIdsArray = Array.isArray(parsed) ? parsed : []; | |
| } | |
| if (!connectedWalletIdsArray.includes(wallet.id)) { | |
| connectedWalletIdsArray.push(wallet.id); | |
| rawStorage.setItem( | |
| connectedWalletIdsKey, | |
| JSON.stringify(connectedWalletIdsArray), | |
| ); | |
| } | |
| rawStorage.setItem(activeWalletIdKey, wallet.id); | |
| } catch (error) { | |
| // Reset on parse error | |
| rawStorage.setItem( | |
| connectedWalletIdsKey, | |
| JSON.stringify([wallet.id]), | |
| ); | |
| rawStorage.setItem(activeWalletIdKey, wallet.id); | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/wagmi-adapter/src/connector.ts around lines 167 to 187: the
JSON.parse call can throw and the read-modify-write logic is verbose and
race-prone; wrap the parse in a try/catch and fall back to an empty array on
error, then simplify to a single read+compute+write cycle that deduplicates
using a Set (read the item, try parse or use [], add wallet.id, stringify and
setItem once), and always set activeWalletId after updating storage; keep
nesting flat and ensure any parse error is swallowed or logged but does not
throw.
| isAuthorized: async () => { | ||
| const connectedWalletIds = rawStorage?.getItem(connectedWalletIdsKey); | ||
| if (connectedWalletIds) { | ||
| const connectedWalletIdsArray = JSON.parse(connectedWalletIds); | ||
| if (connectedWalletIdsArray.includes(wallet.id)) { | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| }, |
There was a problem hiding this comment.
Add error handling to prevent authorization check failures.
The JSON.parse at line 233 can throw on corrupted localStorage data, causing the authorization check to fail entirely instead of returning false.
Apply this diff to add error handling:
isAuthorized: async () => {
+ try {
const connectedWalletIds = rawStorage?.getItem(connectedWalletIdsKey);
if (connectedWalletIds) {
const connectedWalletIdsArray = JSON.parse(connectedWalletIds);
- if (connectedWalletIdsArray.includes(wallet.id)) {
+ if (Array.isArray(connectedWalletIdsArray) && connectedWalletIdsArray.includes(wallet.id)) {
return true;
}
}
+ } catch (error) {
+ // Return false on parse error
+ }
return false;
},📝 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.
| isAuthorized: async () => { | |
| const connectedWalletIds = rawStorage?.getItem(connectedWalletIdsKey); | |
| if (connectedWalletIds) { | |
| const connectedWalletIdsArray = JSON.parse(connectedWalletIds); | |
| if (connectedWalletIdsArray.includes(wallet.id)) { | |
| return true; | |
| } | |
| } | |
| return false; | |
| }, | |
| isAuthorized: async () => { | |
| try { | |
| const connectedWalletIds = rawStorage?.getItem(connectedWalletIdsKey); | |
| if (connectedWalletIds) { | |
| const connectedWalletIdsArray = JSON.parse(connectedWalletIds); | |
| if (Array.isArray(connectedWalletIdsArray) && connectedWalletIdsArray.includes(wallet.id)) { | |
| return true; | |
| } | |
| } | |
| } catch (error) { | |
| // Return false on parse error | |
| } | |
| return false; | |
| }, |
🤖 Prompt for AI Agents
In packages/wagmi-adapter/src/connector.ts around lines 230 to 239, the
authorization check uses JSON.parse on localStorage data which can throw on
corrupted input; wrap the JSON.parse in a try/catch, return false on any parse
error, and ensure the parsed value is an array before calling includes (use
Array.isArray), so the function safely returns false for corrupt or unexpected
data instead of throwing.
3fed520 to
8580fe4
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (7)
packages/wagmi-adapter/src/connector.ts (4)
23-34: ThewithCapabilitiesfield type still has the redundant| boolean.The prior review flagged that
withCapabilities?: withCapabilities | boolean | undefineddefeats the purpose of the generic parameter by widening the type. This issue remains unresolved.Apply this diff to fix the type:
- withCapabilities?: withCapabilities | boolean | undefined; + withCapabilities?: withCapabilities;
144-150: The early return still doesn't respectwithCapabilities.When an account is already connected, this path always returns plain addresses regardless of the
withCapabilitiesparameter. This was flagged in the previous review and remains unfixed.Apply this diff to handle capabilities correctly:
const alreadyConnectedAccount = wallet.getAccount(); if (alreadyConnectedAccount) { return { - accounts: [getAddress(alreadyConnectedAccount.address)], + accounts: (params?.withCapabilities + ? [{ address: getAddress(alreadyConnectedAccount.address), capabilities: {} }] + : [getAddress(alreadyConnectedAccount.address)]) as withCapabilities extends true + ? readonly { address: `0x${string}`; capabilities: Record<string, unknown> }[] + : readonly `0x${string}`[], chainId: wallet.getChain()?.id || 1, }; }
173-193: Add error handling for JSON parsing and simplify the storage logic.The previous review identified that
JSON.parse(lines 176, 239) can throw on corrupted data and that the read-modify-write pattern is susceptible to race conditions. These issues remain unaddressed.Apply this diff to add error handling:
if (rawStorage) { - const connectedWalletIds = rawStorage?.getItem(connectedWalletIdsKey); - if (connectedWalletIds) { - const connectedWalletIdsArray = JSON.parse(connectedWalletIds); - if (Array.isArray(connectedWalletIdsArray)) { - if (!connectedWalletIdsArray.includes(wallet.id)) { - connectedWalletIdsArray.push(wallet.id); - rawStorage.setItem( - connectedWalletIdsKey, - JSON.stringify(connectedWalletIdsArray), - ); - } - } - } else { - rawStorage.setItem( - connectedWalletIdsKey, - JSON.stringify([wallet.id]), - ); + try { + const connectedWalletIds = rawStorage.getItem(connectedWalletIdsKey); + let connectedWalletIdsArray: string[] = []; + + if (connectedWalletIds) { + const parsed = JSON.parse(connectedWalletIds); + connectedWalletIdsArray = Array.isArray(parsed) ? parsed : []; + } + + if (!connectedWalletIdsArray.includes(wallet.id)) { + connectedWalletIdsArray.push(wallet.id); + rawStorage.setItem( + connectedWalletIdsKey, + JSON.stringify(connectedWalletIdsArray), + ); + } + + rawStorage.setItem(activeWalletIdKey, wallet.id); + } catch (error) { + // Reset on parse error + rawStorage.setItem( + connectedWalletIdsKey, + JSON.stringify([wallet.id]), + ); + rawStorage.setItem(activeWalletIdKey, wallet.id); } - rawStorage.setItem(activeWalletIdKey, wallet.id); }
236-245: Add error handling to prevent authorization check failures.The
JSON.parseat line 239 can throw on corrupted localStorage data, causing the authorization check to fail entirely. This was flagged in the previous review and remains unresolved.Apply this diff to add error handling:
isAuthorized: async () => { + try { const connectedWalletIds = rawStorage?.getItem(connectedWalletIdsKey); if (connectedWalletIds) { const connectedWalletIdsArray = JSON.parse(connectedWalletIds); - if (connectedWalletIdsArray.includes(wallet.id)) { + if (Array.isArray(connectedWalletIdsArray) && connectedWalletIdsArray.includes(wallet.id)) { return true; } } + } catch (error) { + // Return false on parse error + } return false; },apps/wagmi-demo/package.json (3)
24-31: Movebufferto runtime dependencies.
bufferis imported insrc/main.tsxto patchglobalThis.Buffer, so omitting it from production installs will break the app. Please move"buffer": "^6.0.3"under"dependencies"and remove it from"devDependencies".
24-31: Pin@wagmi/clito a concrete version.Keeping
"@wagmi/cli": "latest"makes builds nondeterministic and can introduce breaking changes silently. Pin it to the desired released version (e.g."@wagmi/cli": "2.1.0"or newer) and update the lockfile.
30-31: Replace the unpublished Vite version.
"vite": "7.1.7"is not published on npm yet, so installs will fail. Please switch to the latest stable release (currently"7.1.4") or another published tag.
🧹 Nitpick comments (3)
apps/wagmi-demo/.env.example (1)
1-1: Add a trailing newline for POSIX compliance.POSIX text files should end with a newline character. This is a minor formatting issue but improves cross-tool compatibility.
Apply this diff:
-VITE_THIRDWEB_CLIENT_ID= +VITE_THIRDWEB_CLIENT_ID= +apps/wagmi-demo/index.html (1)
6-6: Consider updating the generic title.The title "Create Wagmi" appears to be a placeholder from scaffolding. Consider updating it to something more descriptive like "Thirdweb Wagmi Demo" to better reflect the app's purpose.
Apply this diff:
- <title>Create Wagmi</title> + <title>Thirdweb Wagmi Demo</title>apps/wagmi-demo/package.json (1)
1-33: Add asize-limitbudget.Per repo guidelines, bundle budgets should be tracked via
package.json#size-limit. Please add an appropriatesize-limitsection (or reference an existing shared config) so this app is covered.
📜 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 ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
.changeset/grumpy-toes-switch.md(1 hunks).changeset/quiet-gifts-lead.md(1 hunks)apps/wagmi-demo/.env.example(1 hunks)apps/wagmi-demo/.gitignore(1 hunks)apps/wagmi-demo/.npmrc(1 hunks)apps/wagmi-demo/README.md(1 hunks)apps/wagmi-demo/biome.json(1 hunks)apps/wagmi-demo/index.html(1 hunks)apps/wagmi-demo/package.json(1 hunks)apps/wagmi-demo/src/App.tsx(1 hunks)apps/wagmi-demo/src/index.css(1 hunks)apps/wagmi-demo/src/main.tsx(1 hunks)apps/wagmi-demo/src/vite-env.d.ts(1 hunks)apps/wagmi-demo/src/wagmi.ts(1 hunks)apps/wagmi-demo/tsconfig.base.json(1 hunks)apps/wagmi-demo/tsconfig.json(1 hunks)apps/wagmi-demo/vite.config.ts(1 hunks)package.json(1 hunks)packages/thirdweb/package.json(3 hunks)packages/thirdweb/src/adapters/viem.ts(2 hunks)packages/thirdweb/src/event/actions/get-events.ts(1 hunks)packages/thirdweb/src/exports/react.native.ts(1 hunks)packages/thirdweb/src/exports/react.ts(1 hunks)packages/thirdweb/src/transaction/actions/estimate-gas.ts(1 hunks)packages/wagmi-adapter/package.json(2 hunks)packages/wagmi-adapter/src/connector.ts(6 hunks)packages/wagmi-adapter/src/exports/thirdweb.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/quiet-gifts-lead.md
🚧 Files skipped from review as they are similar to previous changes (15)
- apps/wagmi-demo/.npmrc
- apps/wagmi-demo/src/App.tsx
- apps/wagmi-demo/tsconfig.json
- packages/thirdweb/src/event/actions/get-events.ts
- apps/wagmi-demo/src/index.css
- apps/wagmi-demo/.gitignore
- apps/wagmi-demo/src/vite-env.d.ts
- packages/thirdweb/src/exports/react.native.ts
- packages/thirdweb/src/exports/react.ts
- packages/wagmi-adapter/src/exports/thirdweb.ts
- apps/wagmi-demo/vite.config.ts
- apps/wagmi-demo/tsconfig.base.json
- apps/wagmi-demo/src/main.tsx
- apps/wagmi-demo/src/wagmi.ts
- apps/wagmi-demo/biome.json
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{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/adapters/viem.tspackages/thirdweb/src/transaction/actions/estimate-gas.tspackages/wagmi-adapter/src/connector.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/adapters/viem.tspackages/thirdweb/src/transaction/actions/estimate-gas.tspackages/wagmi-adapter/src/connector.ts
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/adapters/viem.tspackages/thirdweb/src/transaction/actions/estimate-gas.ts
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Track bundle budgets via
package.json#size-limit
Files:
apps/wagmi-demo/package.jsonpackage.jsonpackages/wagmi-adapter/package.jsonpackages/thirdweb/package.json
package.json
📄 CodeRabbit inference engine (CLAUDE.md)
Track bundle budgets via
package.json#size-limit
Files:
package.json
packages/wagmi-adapter/**
📄 CodeRabbit inference engine (CLAUDE.md)
Wagmi ecosystem integration is in
packages/wagmi-adapter/
Files:
packages/wagmi-adapter/package.jsonpackages/wagmi-adapter/src/connector.ts
.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/grumpy-toes-switch.md
🧠 Learnings (3)
📚 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/adapters/viem.ts
📚 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: Build all packages using `pnpm build` and specific packages with dependencies using `turbo run build --filter=./packages/*`
Applied to files:
package.json
📚 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/wagmi-adapter/** : Wagmi ecosystem integration is in `packages/wagmi-adapter/`
Applied to files:
packages/wagmi-adapter/package.jsonapps/wagmi-demo/README.md
🧬 Code graph analysis (1)
packages/wagmi-adapter/src/connector.ts (2)
packages/wagmi-adapter/src/exports/thirdweb.ts (1)
ConnectionOptions(2-2)apps/wagmi-demo/src/wagmi.ts (2)
client(14-16)config(21-35)
🪛 dotenv-linter (3.3.0)
apps/wagmi-demo/.env.example
[warning] 1-1: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
⏰ 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). (6)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (11)
packages/thirdweb/src/adapters/viem.ts (1)
435-436: LGTM! Good refactor for clarity and efficiency.The change to store the account locally and perform an explicit null check improves both readability and efficiency by avoiding multiple calls to
wallet.getAccount(). This pattern is consistent with the deprecatedtoViemWalletClientfunction (lines 310, 379) and maintains the same error-handling semantics.Also applies to: 460-460
packages/thirdweb/src/transaction/actions/estimate-gas.ts (1)
120-126: LGTM: mapping toaddressaligns with SignedAuthorization’saddressfield.apps/wagmi-demo/README.md (1)
1-38: LGTM!The README provides clear documentation for the wagmi-demo app, including a helpful code snippet and setup instructions. The content is consistent with the implementation.
packages/wagmi-adapter/src/connector.ts (1)
110-121: Approve the refactored storage and chain ID parsing.The changes to move
rawStorageto the top level and parselastChainIdas a number improve clarity and consistency.packages/wagmi-adapter/package.json (2)
46-46: LGTM on the new dev script!The addition of a watch-mode TypeScript compilation script improves the development experience for the wagmi-adapter package.
8-8: Approve @wagmi/core updateVersion 2.21.2 is the latest on npm and has no open security advisories.
.changeset/grumpy-toes-switch.md (1)
1-6: LGTM!The changeset correctly documents the patch update for
@thirdweb-dev/wagmi-adapterwith an accurate description of the changes.package.json (1)
68-68: LGTM!The new
wagmi-demoscript is well-structured and follows the established pattern for running demo apps with their dependencies.packages/thirdweb/package.json (3)
70-70: Approve version bump to @vitejs/plugin-react@5.0.4 (latest release, no reported security advisories)
42-42: No issues with viem update
viem@2.37.9 is the latest version on npm and has no reported security advisories.
97-97: No action: vite@7.1.7 is the latest release and not affected by known advisories
joaquim-verges
deleted the
10-01-update_dependencies_improve_wagmi_adapter
branch
PR-Codex overview
This PR focuses on updating the
wagmi-demoapplication and its dependencies, enhancing wallet connection features, and improving configuration and styling.Detailed summary
viteclient reference invite-env.d.ts.legacy-peer-depsto true in.npmrc..changeset.VITE_THIRDWEB_CLIENT_IDin.env.example.thirdwebwallet connection management.vite.config.tsto define server port.index.css.index.html.package.jsonscripts and dependencies.App.tsxwith wallet connection features.tsconfig.jsonandtsconfig.base.json.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores