Skip to content

Conversation

@ignaciosantise
Copy link
Collaborator

@ignaciosantise ignaciosantise commented Nov 5, 2025

This pull request introduces a new hook, useAppKitTheme, to the AppKit React Native ecosystem, enabling developers to control and customize the theme mode (light/dark) and accent color of the AppKit modal. The hook provides an interface for reading and updating theme settings, improving theming flexibility and integration with host applications.

The most important changes are:

New Feature: Theme Control Hook

  • Added the useAppKitTheme hook in packages/appkit/src/hooks/useAppKitTheme.ts, allowing consumers to get and set the current theme mode and accent color for AppKit modals. The hook provides a typed API and is documented with usage examples.

Note

Introduce useAppKitTheme and a shared useAppKitContext, refactor hooks to use it, and enhance ThemeController with defaults and tests.

  • AppKit (React Native)
    • New Hook: Add useAppKitTheme (packages/appkit/src/hooks/useAppKitTheme.ts) and export from src/index.ts.
    • Context: Add useAppKitContext and refactor useAccount, useAppKit, useAppKitEvents, useAppKitLogs, useAppKitState, useProvider, useWalletInfo to use it for provider-scope checks.
    • AppKitContext: Export AppKitContextType and keep provider wiring.
    • Tests/Config: Add jest.config.ts, jest-setup.ts, and babel.config.js; add tests for useAppKitTheme.
  • Core
    • ThemeController: Make themeVariables non-optional; add defaultThemeMode; update setThemeMode to fallback to system (default light); support resetting setThemeVariables; add/expand tests.
  • Repo
    • Scripts: Remove build step from root test script; add test script to packages/appkit/package.json.
    • Changeset: Patch bump across @reown/* packages.

Written by Cursor Bugbot for commit 32e7408. This will update automatically on new commits. Configure here.

@changeset-bot
Copy link

changeset-bot bot commented Nov 5, 2025

🦋 Changeset detected

Latest commit: 32e7408

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@reown/appkit-react-native Patch
@reown/appkit-ui-react-native Patch
@reown/appkit-bitcoin-react-native Patch
@reown/appkit-coinbase-react-native Patch
@reown/appkit-common-react-native Patch
@reown/appkit-core-react-native Patch
@reown/appkit-ethers-react-native Patch
@reown/appkit-solana-react-native Patch
@reown/appkit-wagmi-react-native Patch

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

@claude
Copy link

claude bot commented Nov 5, 2025

Claude finished @ignaciosantise's task —— View job


Code Review Complete

Tasks:

  • Read repository guidelines (CLAUDE.md)
  • Review hook implementation file
  • Review changeset file
  • Analyze code quality and best practices
  • Check for bugs and security issues
  • Provide comprehensive feedback

Issues Found

🔴 Critical: Hook Not Exported

File: packages/appkit/src/index.ts

The useAppKitTheme hook is implemented but not exported from the package. Users cannot access it.

Fix required: Add export to packages/appkit/src/index.ts:38:

export { useAppKitTheme } from './hooks/useAppKitTheme';

🟡 Bug: Incorrect ThemeController Behavior

File: packages/appkit/src/hooks/useAppKitTheme.ts:65-66

The hook calls useAppKit() for provider validation, but useAppKit() returns functions that are unused. This works but is semantically incorrect - it suggests the hook depends on AppKit instance methods when it only needs provider context validation.

Issue: ThemeController is a standalone Valtio proxy that doesn't require the AppKit instance. Calling useAppKit() creates unnecessary coupling.

Recommendation: Either:

  1. Create a lightweight useAppKitContext() hook specifically for context validation, or
  2. Directly use useContext(AppKitContext) with error handling in this hook

🟡 Missing Tests

Location: No test file exists for useAppKitTheme

Per project standards requiring 80%+ test coverage, this hook needs tests. The existing ThemeController.test.ts only tests the controller, not the React hook.

Required test cases:

  • Hook throws error when used outside AppKitProvider
  • Hook returns correct theme mode and variables from controller state
  • setThemeMode updates controller state
  • setThemeVariables updates controller state
  • Setter functions maintain referential stability (don't cause re-renders)
  • Hook updates when controller state changes

Reference: See packages/core/src/__tests__/controllers/ThemeController.test.ts for controller tests

🟢 Minor: Inconsistent Empty Value Handling

File: packages/appkit/src/hooks/useAppKitTheme.ts:68-73

The hook uses useMemo with empty dependency array [] to stabilize setter functions. However, examining ThemeController.setThemeVariables (packages/core/src/controllers/ThemeController.ts:40-46), there's inconsistent behavior:

setThemeVariables(themeVariables: ThemeControllerState['themeVariables']) {
  if (!themeVariables) {
    state.themeVariables = {};
  }
  state.themeVariables = { ...state.themeVariables, ...themeVariables };
}

When themeVariables is undefined, it sets to {}, but then still spreads undefined on line 45, which is a no-op but semantically confusing.

Controller fix needed:

setThemeVariables(themeVariables: ThemeControllerState['themeVariables']) {
  if (!themeVariables) {
    state.themeVariables = {};
    return;
  }
  state.themeVariables = { ...state.themeVariables, ...themeVariables };
}

🟢 Documentation: Missing Type Export

File: packages/appkit/src/index.ts

The hook exports UseAppKitThemeReturn interface, but it's not re-exported from the package index. Users cannot import the type for their own type annotations.

Recommendation: Export the type:

export { useAppKitTheme, type UseAppKitThemeReturn } from './hooks/useAppKitTheme';

This matches the pattern used for other hooks like useAccount (line 34).


Summary

Blocking issues:

  1. ❌ Hook is not exported - users cannot use it
  2. ⚠️ No tests - fails project's 80% coverage requirement

Non-blocking issues:
3. Unnecessary coupling to useAppKit()
4. Controller logic inconsistency in setThemeVariables
5. Missing type export for TypeScript users

The implementation is well-documented and follows React hooks best practices (proper use of useMemo, useSnapshot, function binding). However, the export and testing gaps must be addressed before merge.


@socket-security
Copy link

socket-security bot commented Nov 5, 2025

@ignaciosantise
Copy link
Collaborator Author

@claude review

@claude
Copy link

claude bot commented Nov 14, 2025

Claude encountered an error —— View job


I'll analyze this and get back to you.

Base automatically changed from fix/use-account-clear to develop November 14, 2025 13:50
Copilot AI review requested due to automatic review settings November 14, 2025 13:55
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new useAppKitTheme hook for controlling theme mode and accent color in AppKit React Native modals. It also refactors context handling by introducing a centralized useAppKitContext hook and updates the ThemeController to use undefined as the default for themeVariables.

Key changes:

  • Added useAppKitTheme hook with getters/setters for theme mode and variables
  • Created useAppKitContext to centralize AppKit context validation
  • Updated ThemeController default state from {} to undefined for themeVariables

Reviewed Changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/core/src/controllers/ThemeController.ts Changed default themeVariables from empty object to undefined; updated setThemeVariables to clear variables when passed undefined
packages/core/src/tests/controllers/ThemeController.test.ts Updated tests to reflect undefined default for themeVariables
packages/appkit/src/index.ts Exported new useAppKitTheme hook and its return type
packages/appkit/src/hooks/useAppKitTheme.ts New hook providing theme mode and variables control with proper memoization
packages/appkit/src/hooks/useAppKitContext.ts New internal hook for centralized context validation
packages/appkit/src/hooks/useAppKit.ts Refactored to use useAppKitContext; added comprehensive documentation
packages/appkit/src/hooks/useAccount.ts Updated to use useAppKitContext instead of useAppKit
packages/appkit/src/hooks/useProvider.ts Updated to use useAppKitContext; simplified documentation example
packages/appkit/src/hooks/useWalletInfo.ts Updated to use useAppKitContext; added comprehensive documentation
packages/appkit/src/hooks/useAppKitState.ts Updated to use useAppKitContext; added comprehensive documentation
packages/appkit/src/hooks/useAppKitEvents.ts Updated to use useAppKitContext; added comprehensive documentation
packages/appkit/src/hooks/useAppKitLogs.ts Updated to use useAppKitContext; removed duplicate context validation
packages/appkit/src/tests/hooks/useAppKitTheme.test.tsx Comprehensive test suite for useAppKitTheme hook
packages/appkit/src/AppKitContext.tsx Exported AppKitContextType interface
packages/appkit/package.json Added test script
packages/appkit/jest.config.ts Jest configuration for appkit package
packages/appkit/jest-setup.ts Jest setup importing shared configuration
packages/appkit/babel.config.js Babel configuration for React Native preset
package.json Updated test script to remove turbo build dependency
.changeset/spotty-foxes-fry.md Changeset documenting patch bumps for affected packages

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

@sonarqubecloud
Copy link

@ignaciosantise ignaciosantise merged commit 048885f into develop Nov 14, 2025
10 of 11 checks passed
@ignaciosantise ignaciosantise deleted the chore/use-theme-hook branch November 14, 2025 15:19
@github-actions github-actions bot locked and limited conversation to collaborators Nov 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants