fix(core): immutable state updates in store mutation functions (#220)

* fix(use-wallet): create new object references in setActiveAccount

* fix: immutable state updates in addWallet and setAccounts

* refactor: store mutation function improvements

* chore(use-wallet): fix import sort order in src/store.ts

packages/use-wallet/src/__tests__/store.test.ts

@@ -42,6 +42,43 @@ describe('Mutations', () => {
       expect(state.wallets[walletId]).toEqual(walletState)
       expect(state.activeWallet).toBe(walletId)
     })
+
+    it('should create new object references when adding a wallet', () => {
+      const walletId = WalletId.DEFLY
+      const account1 = {
+        name: 'Defly Wallet 1',
+        address: 'address1'
+      }
+      const account2 = {
+        name: 'Defly Wallet 2',
+        address: 'address2'
+      }
+      const walletState = {
+        accounts: [account1, account2],
+        activeAccount: account1
+      }
+
+      const originalWalletState = { ...walletState }
+
+      addWallet(store, { walletId, wallet: walletState })
+
+      const storedWallet = store.state.wallets[walletId]
+
+      // Check that new object references were created
+      expect(storedWallet).not.toBe(walletState)
+      expect(storedWallet?.accounts).not.toBe(walletState.accounts)
+      expect(storedWallet?.activeAccount).not.toBe(walletState.activeAccount)
+
+      // Check that the content is still correct
+      expect(storedWallet?.accounts).toEqual([account1, account2])
+      expect(storedWallet?.activeAccount).toEqual(account1)
+
+      // Modify the stored wallet state
+      storedWallet!.accounts[0].name = 'Modified Name'
+
+      // Check that the original wallet state is unchanged
+      expect(walletState).toEqual(originalWalletState)
+    })
   })
 
   describe('removeWallet', () => {
@@ -218,6 +255,73 @@ describe('Mutations', () => {
       setActiveAccount(store, { walletId: WalletId.DEFLY, address: 'foo' })
       expect(store.state.wallets[walletId]?.activeAccount).toEqual(account1)
     })
+
+    it('should not modify other accounts when setting active account', () => {
+      const walletId = WalletId.DEFLY
+      const account1 = {
+        name: 'Defly Wallet 1',
+        address: 'address1'
+      }
+      const account2 = {
+        name: 'Defly Wallet 2',
+        address: 'address2'
+      }
+      const walletState = {
+        accounts: [account1, account2],
+        activeAccount: account1
+      }
+
+      addWallet(store, { walletId, wallet: walletState })
+      expect(store.state.wallets[walletId]?.accounts).toEqual([account1, account2])
+      expect(store.state.wallets[walletId]?.activeAccount).toEqual(account1)
+
+      setActiveAccount(store, { walletId, address: account2.address })
+
+      // Check that active account has changed
+      expect(store.state.wallets[walletId]?.activeAccount).toEqual(account2)
+
+      // Check that accounts array is unchanged
+      expect(store.state.wallets[walletId]?.accounts).toEqual([account1, account2])
+
+      // Verify that the first account in the array is still account1
+      expect(store.state.wallets[walletId]?.accounts[0]).toEqual(account1)
+    })
+
+    it('should create new object references for active account and accounts array', () => {
+      const walletId = WalletId.DEFLY
+      const account1 = {
+        name: 'Defly Wallet 1',
+        address: 'address1'
+      }
+      const account2 = {
+        name: 'Defly Wallet 2',
+        address: 'address2'
+      }
+      const walletState = {
+        accounts: [account1, account2],
+        activeAccount: account1
+      }
+
+      addWallet(store, { walletId, wallet: walletState })
+      const originalWallet = store.state.wallets[walletId]
+      const originalAccounts = originalWallet?.accounts
+      const originalActiveAccount = originalWallet?.activeAccount
+
+      setActiveAccount(store, { walletId, address: account2.address })
+
+      const updatedWallet = store.state.wallets[walletId]
+      const updatedAccounts = updatedWallet?.accounts
+      const updatedActiveAccount = updatedWallet?.activeAccount
+
+      // Check that new object references were created
+      expect(updatedWallet).not.toBe(originalWallet)
+      expect(updatedAccounts).not.toBe(originalAccounts)
+      expect(updatedActiveAccount).not.toBe(originalActiveAccount)
+
+      // Check that the content is still correct
+      expect(updatedAccounts).toEqual([account1, account2])
+      expect(updatedActiveAccount).toEqual(account2)
+    })
   })
 
   describe('setAccounts', () => {
@@ -273,6 +377,47 @@ describe('Mutations', () => {
       // Active account should be set to first account in new accounts list (account2)
       expect(store.state.wallets[walletId]?.activeAccount).toEqual(account2)
     })
+
+    it('should create new object references when setting accounts', () => {
+      const walletId = WalletId.DEFLY
+      const account1 = {
+        name: 'Defly Wallet 1',
+        address: 'address1'
+      }
+      const account2 = {
+        name: 'Defly Wallet 2',
+        address: 'address2'
+      }
+      const walletState = {
+        accounts: [account1],
+        activeAccount: account1
+      }
+
+      addWallet(store, { walletId, wallet: walletState })
+
+      const newAccounts = [account1, account2]
+      const originalNewAccounts = [...newAccounts]
+
+      setAccounts(store, { walletId, accounts: newAccounts })
+
+      const storedWallet = store.state.wallets[walletId]
+
+      // Check that new object references were created
+      expect(storedWallet?.accounts).not.toBe(newAccounts)
+      expect(storedWallet?.accounts[0]).not.toBe(account1)
+      expect(storedWallet?.accounts[1]).not.toBe(account2)
+      expect(storedWallet?.activeAccount).not.toBe(account1)
+
+      // Check that the content is still correct
+      expect(storedWallet?.accounts).toEqual([account1, account2])
+      expect(storedWallet?.activeAccount).toEqual(account1)
+
+      // Modify the stored accounts
+      storedWallet!.accounts[0].name = 'Modified Name'
+
+      // Check that the original new accounts array is unchanged
+      expect(newAccounts).toEqual(originalNewAccounts)
+    })
   })
 
   describe('setActiveNetwork', () => {

packages/use-wallet/src/store.ts

@@ -1,7 +1,7 @@
+import { Algodv2 } from 'algosdk'
 import { NetworkId, isValidNetworkId } from 'src/network'
 import { WalletId, type WalletAccount } from 'src/wallets'
 import type { Store } from '@tanstack/store'
-import { Algodv2 } from 'algosdk'
 
 export type WalletState = {
   accounts: WalletAccount[]
@@ -33,39 +33,40 @@ export function addWallet(
   { walletId, wallet }: { walletId: WalletId; wallet: WalletState }
 ) {
   store.setState((state) => {
-    const newWallets = {
+    const updatedWallets = {
       ...state.wallets,
-      [walletId]: wallet
+      [walletId]: {
+        accounts: wallet.accounts.map((account) => ({ ...account })),
+        activeAccount: wallet.activeAccount ? { ...wallet.activeAccount } : null
+      }
     }
 
     return {
       ...state,
-      wallets: newWallets,
+      wallets: updatedWallets,
       activeWallet: walletId
     }
   })
 }
 
 export function removeWallet(store: Store<State>, { walletId }: { walletId: WalletId }) {
   store.setState((state) => {
-    const newWallets = { ...state.wallets }
-    delete newWallets[walletId]
+    const updatedWallets = { ...state.wallets }
+    delete updatedWallets[walletId]
 
     return {
       ...state,
-      wallets: newWallets,
+      wallets: updatedWallets,
       activeWallet: state.activeWallet === walletId ? null : state.activeWallet
     }
   })
 }
 
 export function setActiveWallet(store: Store<State>, { walletId }: { walletId: WalletId | null }) {
-  store.setState((state) => {
-    return {
-      ...state,
-      activeWallet: walletId
-    }
-  })
+  store.setState((state) => ({
+    ...state,
+    activeWallet: walletId
+  }))
 }
 
 export function setActiveAccount(
@@ -75,24 +76,30 @@ export function setActiveAccount(
   store.setState((state) => {
     const wallet = state.wallets[walletId]
     if (!wallet) {
+      console.warn(`Wallet with id "${walletId}" not found`)
       return state
     }
-    const activeAccount = wallet.accounts.find((a) => a.address === address)
-    if (!activeAccount) {
+
+    const newActiveAccount = wallet.accounts.find((a) => a.address === address)
+    if (!newActiveAccount) {
+      console.warn(`Account with address ${address} not found in wallet "${walletId}"`)
       return state
     }
 
-    const newWallets = {
+    const updatedWallet = {
+      ...wallet,
+      accounts: wallet.accounts.map((account) => ({ ...account })),
+      activeAccount: { ...newActiveAccount }
+    }
+
+    const updatedWallets = {
       ...state.wallets,
-      [walletId]: {
-        ...wallet,
-        activeAccount
-      }
+      [walletId]: updatedWallet
     }
 
     return {
       ...state,
-      wallets: newWallets
+      wallets: updatedWallets
     }
   })
 }
@@ -104,42 +111,43 @@ export function setAccounts(
   store.setState((state) => {
     const wallet = state.wallets[walletId]
     if (!wallet) {
+      console.warn(`Wallet with id "${walletId}" not found`)
       return state
     }
 
-    // Check if `accounts` includes `wallet.activeAccount`
-    const isActiveAccountConnected = accounts.some(
+    const newAccounts = accounts.map((account) => ({ ...account }))
+
+    const isActiveAccountConnected = newAccounts.some(
       (account) => account.address === wallet.activeAccount?.address
     )
 
-    const activeAccount = isActiveAccountConnected ? wallet.activeAccount! : accounts[0] || null
+    const newActiveAccount = isActiveAccountConnected
+      ? { ...wallet.activeAccount! }
+      : newAccounts[0] || null
 
-    const newWallet = {
+    const updatedWallet = {
       ...wallet,
-      accounts,
-      activeAccount
+      accounts: newAccounts,
+      activeAccount: newActiveAccount
     }
 
-    // Create a new map with the updated wallet
-    const newWallets = {
+    const updatedWallets = {
       ...state.wallets,
-      [walletId]: newWallet
+      [walletId]: updatedWallet
     }
 
     return {
       ...state,
-      wallets: newWallets
+      wallets: updatedWallets
     }
   })
 }
 
 export function setActiveNetwork(store: Store<State>, { networkId }: { networkId: NetworkId }) {
-  store.setState((state) => {
-    return {
-      ...state,
-      activeNetwork: networkId
-    }
-  })
+  store.setState((state) => ({
+    ...state,
+    activeNetwork: networkId
+  }))
 }
 
 // Type guards