fix: clear old uri before creating a new one on connecting view

[ignaciosantise]

This pull request introduces a small but important improvement to the ConnectingView component. The change ensures that any stale WalletConnect URI from previous connection attempts is cleared before initializing a new connection, which helps prevent connection issues.

• packages/appkit/src/views/w3m-connecting-view/index.tsx: Added a call to WcController.clearUri() at the start of the useEffect to clear stale URIs before initializing a new connection.

---

Note

Clears old WalletConnect URI before reconnecting, switches retry timing to a ref, and forces initial reconnect with updated QR auto-retry logic.

• ConnectingView (packages/appkit/src/views/w3m-connecting-view/index.tsx)
  • Clear stale wc URI before establishing a new connection on retry or expiry.
  • Refactor retry timing: replace lastRetry state with useRef, updating debounce and QR auto-retry logic.
  • Simplify initializeConnection (remove extra arg) and invoke initializeConnection(true) on mount; keep periodic pairing-expiry checks.

^{Written by Cursor Bugbot for commit 5cb1e29. This will update automatically on new commits. Configure here.}

[changeset-bot]

⚠️ No Changeset found

Latest commit: 5cb1e29

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[claude]

Claude finished @ignaciosantise's task —— View job

---

Code Review

• Read repository guidelines (CLAUDE.md)
• Review the changed file
• Analyze WcController implementation
• Check for test coverage
• Provide final review feedback

---

Issues Found

1. Potential useEffect Infinite Loop Risk (packages/appkit/src/views/w3m-connecting-view/index.tsx:109-123)

The useEffect at line 109 has a missing dependency that could cause issues:

useEffect(() => {
  WcController.clearUri();
  initializeConnection();
  // ...
}, [isQr]); // Missing 'initializeConnection' in dependencies

Problem: The initializeConnection function is called inside the effect but not listed in the dependency array. While the eslint disable comment on line 122 suppresses the warning, this could lead to:

• Using stale closure values (e.g., lastRetry, connect, data)
• Unpredictable behavior when those values change

Recommendation: Either:

1. Wrap initializeConnection with useCallback and include it in dependencies
2. Move the function inside the useEffect if it doesn't need to be called elsewhere
3. Document why the specific dependencies are safe to omit

2. Missing Changeset (Required)

The PR is missing a changeset, which is required for version tracking. Since this is a bug fix that clears stale URIs to prevent connection issues, it should have a patch changeset.

Action needed: Add a changeset by running:

npx changeset add

Select @reown/appkit-react-native as patch and use a message like: "fix: clear old uri before creating a new one on connecting view"

3. Redundant URI Clearing (Minor Code Quality Issue)

The clearUri() is now called in three places:

• Line 111: At the start of the useEffect (new)
• Line 48: Inside initializeConnection when retry or pairing expired
• Line 32-35 in onRetry: Indirectly via initializeConnection(true)

Analysis: The new call at line 111 ensures URI is always cleared when component mounts or isQr changes. However, this creates overlap with line 48 since initializeConnection() is immediately called after on line 113.

Impact: Low - clearUri() is idempotent (safe to call multiple times), but this is slightly redundant.

Recommendation: Consider if the clearing at line 111 makes the one at line 48 unnecessary for the initial call. The current implementation is safe but could be optimized.

4. Missing Test Coverage

No test file exists for ConnectingView. Given this component handles critical connection logic with multiple edge cases (retries, errors, pairing expiry), it should have comprehensive test coverage.

Recommendation: Add tests covering:

• URI clearing on component mount
• URI clearing on retry
• URI clearing when switching between QR and wallet modes
• Behavior when pairing expires

---

Summary

The change itself is sensible and addresses a real issue with stale URIs. However, there are concerns around the useEffect dependencies that could lead to bugs, and a changeset is required before merging.

Action items before merge:

1. ⚠️ Critical: Add the missing changeset
2. ⚠️ Important: Address the useEffect dependency issue to prevent potential bugs with stale closures
3. 📝 Nice to have: Add test coverage for this component

---

[Copilot]

Pull Request Overview

This PR addresses a bug where stale WalletConnect URIs from previous connection attempts could interfere with new connections in the ConnectingView component. The fix adds a call to WcController.clearUri() at the start of the connection initialization useEffect.

Key Changes:

• Clears stale WalletConnect URI before initializing new connections in ConnectingView

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ignaciosantise]

@claude review

[claude]

Claude finished @ignaciosantise's task —— View job

---

🔄 Incremental Review: Validation of Previous Feedback

---

Critical Issue: Cursor's Bug Report is CONFIRMED

Location: packages/appkit/src/views/w3m-connecting-view/index.tsx:111

The bug identified by Cursor is valid and serious. Here's why:

The Problem Chain:

1. Line 111: WcController.clearUri() clears wcPairingExpiry to undefined
2. Line 113: initializeConnection() is called immediately after
3. Line 45: Inside initializeConnection(), reads wcPairingExpiry from state (now undefined)
4. Line 48-49: isPairingExpired(undefined) returns true (see CoreHelperUtil.ts:49)
5. This forces line 48 to clear URI again and create a new pairing, even if a valid unexpired pairing existed

Code Evidence:

// CoreHelperUtil.ts:48-49
isPairingExpired(expiry?: number) {
  return expiry ? expiry - Date.now() <= ConstantsUtil.TEN_SEC_MS : true;
  //                                                                 ^^^^
  //                                                      Always true when undefined
}

Impact: Every component mount or isQr change bypasses pairing reuse logic, forcing unnecessary new pairings and potentially degrading UX.

Fix Options:

1. Remove line 111 - Let initializeConnection handle clearing when needed (lines 48)
2. Add conditional check - Only clear if truly stale
3. Pass flag to initializeConnection - Indicate this is a "fresh start" requiring URI clear

---

Other Previously Reported Issues - Status Check

✅ Missing Changeset - Still Required

Confirmed: No changeset exists. This is a bug fix requiring a patch changeset.

⚠️ useEffect Dependency Issue - Partially Valid

The useEffect at line 109-123 has initializeConnection in its closure but not in dependencies. However, since initializeConnection is defined outside the effect and uses refs (lastRetryRef), this is actually stable. The eslint-disable on line 122 is reasonable if the function remains stable across renders.

Recommendation: Wrap initializeConnection with useCallback to make dependency intent explicit and remove the eslint-disable.

ℹ️ Redundant Clearing - Now Confirmed Problematic

The "redundant clearing" concern is actually the critical bug described above. This isn't just inefficient—it's breaking pairing reuse logic.

ℹ️ Missing Test Coverage - Still Valid

No test file exists for this component. Given the complexity of connection retry logic and this bug, tests are highly recommended.

---

New Issues Found

State Management: lastRetry → lastRetryRef Migration

Location: Lines 25, 33-34, 76, 79

The conversion to useRef is correct for preventing unnecessary re-renders, but the logic at line 76 is concerning:

const currentRetryTime = retryTimestamp ?? lastRetryRef.current;

If retryTimestamp is provided (line 80), it bypasses the ref entirely. This creates two parallel retry tracking mechanisms. Consider simplifying to always use the ref.

---

Summary

Blocking Issues:

1. 🚨 Critical Bug: Line 111 clearUri() breaks pairing reuse (Cursor's report confirmed)
2. ⚠️ Missing Changeset: Required before merge

Recommended Before Merge:

• Fix the pairing reuse bug (remove or conditionally call line 111)
• Add changeset
• Consider wrapping initializeConnection in useCallback

Nice to Have:

• Add test coverage for connection retry logic
• Simplify retry timestamp tracking

---

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud