Skip to content

Commit

Permalink
fix: deeplink issue after wallet connection
Browse files Browse the repository at this point in the history
revert: removing afterAll in SafeLocalStorage.test.ts

fix: StorageUtil.test.ts

fix: SafeLocalStorage.test.ts

chore: remove setisConnected
  • Loading branch information
magiziz committed Sep 23, 2024
1 parent 08c3393 commit 2a75488
Show file tree
Hide file tree
Showing 12 changed files with 34 additions and 58 deletions.
1 change: 0 additions & 1 deletion packages/adapters/ethers5/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,6 @@ export class Ethers5Adapter {
return
}

this.appKit?.setIsConnected(true, this.chainNamespace)
this.appKit?.setPreferredAccountType(
preferredAccountType as W3mFrameTypes.AccountType,
this.chainNamespace
Expand Down
10 changes: 1 addition & 9 deletions packages/appkit/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,6 @@ export class AppKit {
return NetworkController.switchActiveNetwork(caipNetwork)
}

public getIsConnected() {
return AccountController.state.isConnected
}

public getWalletProvider() {
return ChainController.state.activeChain
? ProviderUtil.state.providers[ChainController.state.activeChain]
Expand Down Expand Up @@ -218,15 +214,11 @@ export class AppKit {
]?.replace
}

public setIsConnected: (typeof AccountController)['setIsConnected'] = (isConnected, chain) => {
AccountController.setIsConnected(isConnected, chain)
}

public setStatus: (typeof AccountController)['setStatus'] = (status, chain) => {
AccountController.setStatus(status, chain)
}

public getIsConnectedState = () => AccountController.state.isConnected
public getIsConnectedState = () => Boolean(ChainController.state.activeCaipAddress)

public setAllAccounts: (typeof AccountController)['setAllAccounts'] = (addresses, chain) => {
AccountController.setAllAccounts(addresses, chain)
Expand Down
11 changes: 1 addition & 10 deletions packages/appkit/src/tests/appkit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,21 +177,11 @@ describe('Base', () => {
expect(appKit.isTransactionShouldReplaceView()).toBe(true)
})

it('should set is connected', () => {
appKit.setIsConnected(true, 'eip155')
expect(AccountController.setIsConnected).toHaveBeenCalledWith(true, 'eip155')
})

it('should set status', () => {
appKit.setStatus('connected', 'eip155')
expect(AccountController.setStatus).toHaveBeenCalledWith('connected', 'eip155')
})

it('should get is connected state', () => {
vi.mocked(AccountController).state = { isConnected: true } as any
expect(appKit.getIsConnectedState()).toBe(true)
})

it('should set all accounts', () => {
const addresses = ['0x123', '0x456'] as unknown as AccountType[]
appKit.setAllAccounts(addresses, 'eip155')
Expand Down Expand Up @@ -239,6 +229,7 @@ describe('Base', () => {

it('should set CAIP address', () => {
appKit.setCaipAddress('eip155:1:0x123', 'eip155')
expect(appKit.getIsConnectedState()).toBe(true)
expect(AccountController.setCaipAddress).toHaveBeenCalledWith('eip155:1:0x123', 'eip155')
})

Expand Down
16 changes: 12 additions & 4 deletions packages/common/src/utils/SafeLocalStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export type SafeLocalStorageItems = {
'@appkit/connected_social': string
'@appkit/connected_social_username': string
'@appkit/recent_wallets': string
'@appkit/deeplink_choice': string
WALLETCONNECT_DEEPLINK_CHOICE: { href: string; name: string }
}

export const SafeLocalStorageKeys = {
Expand All @@ -21,7 +21,7 @@ export const SafeLocalStorageKeys = {
CONNECTED_SOCIAL: '@appkit/connected_social',
CONNECTED_SOCIAL_USERNAME: '@appkit/connected_social_username',
RECENT_WALLETS: '@appkit/recent_wallets',
DEEPLINK_CHOICE: '@appkit/deeplink_choice'
DEEPLINK_CHOICE: 'WALLETCONNECT_DEEPLINK_CHOICE'
} as const

export const SafeLocalStorage = {
Expand All @@ -30,14 +30,22 @@ export const SafeLocalStorage = {
value: SafeLocalStorageItems[Key]
): void {
if (isSafe()) {
localStorage.setItem(key, value)
localStorage.setItem(key, JSON.stringify(value))
}
},
getItem<Key extends keyof SafeLocalStorageItems>(
key: Key
): SafeLocalStorageItems[Key] | undefined {
if (isSafe()) {
return localStorage.getItem(key) || undefined
const value = localStorage.getItem(key)

if (value) {
try {
return JSON.parse(value)
} catch {
return undefined
}
}
}

return undefined
Expand Down
4 changes: 2 additions & 2 deletions packages/common/tests/SafeLocalStorage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ describe('SafeLocalStorage unsafe', () => {
describe('SafeLocalStorage safe', () => {
let getItem = vi.fn(value => {
if (value === '@appkit/wallet_id') {
return 'test'
return JSON.stringify('test')
}

return undefined
Expand All @@ -46,7 +46,7 @@ describe('SafeLocalStorage safe', () => {

it('should setItem', () => {
expect(SafeLocalStorage.setItem('@appkit/wallet_id', 'test')).toBe(undefined)
expect(setItem).toHaveBeenCalledWith('@appkit/wallet_id', 'test')
expect(setItem).toHaveBeenCalledWith('@appkit/wallet_id', JSON.stringify('test'))
})

it('should getItem ', () => {
Expand Down
13 changes: 0 additions & 13 deletions packages/core/src/controllers/AccountController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import type UniversalProvider from '@walletconnect/universal-provider'

// -- Types --------------------------------------------- //
export interface AccountControllerState {
isConnected: boolean
currentTab: number
caipAddress?: CaipAddress
address?: string
Expand All @@ -45,7 +44,6 @@ export interface AccountControllerState {

// -- State --------------------------------------------- //
const state = proxy<AccountControllerState>({
isConnected: false,
currentTab: 0,
tokenBalance: [],
smartAccountDeployed: false,
Expand Down Expand Up @@ -97,21 +95,10 @@ export const AccountController = {
)
},

setIsConnected(
isConnected: AccountControllerState['isConnected'],
chain: ChainNamespace | undefined
) {
ChainController.setAccountProp('isConnected', isConnected, chain)
},

setStatus(status: AccountControllerState['status'], chain: ChainNamespace | undefined) {
ChainController.setAccountProp('status', status, chain)
},

getChainIsConnected(chain: ChainNamespace | undefined) {
return ChainController.getAccountProp('isConnected', chain)
},

getCaipAddress(chain: ChainNamespace | undefined) {
return ChainController.getAccountProp('caipAddress', chain)
},
Expand Down
2 changes: 0 additions & 2 deletions packages/core/src/controllers/ChainController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ type ChainsInitializerAdapter = Pick<

// -- Constants ----------------------------------------- //
const accountState: AccountControllerState = {
isConnected: false,
currentTab: 0,
tokenBalance: [],
smartAccountDeployed: false,
Expand Down Expand Up @@ -449,7 +448,6 @@ export const ChainController = {
this.setChainAccountData(
chainToWrite,
ref({
isConnected: false,
smartAccountDeployed: false,
currentTab: 0,
caipAddress: undefined,
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/utils/StorageUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import { ChainController } from '../controllers/ChainController.js'

// -- Utility -----------------------------------------------------------------
export const StorageUtil = {
setWalletConnectDeepLink({ href }: { href: string; name: string }) {
setWalletConnectDeepLink({ name, href }: { href: string; name: string }) {
try {
SafeLocalStorage.setItem(SafeLocalStorageKeys.DEEPLINK_CHOICE, href)
SafeLocalStorage.setItem(SafeLocalStorageKeys.DEEPLINK_CHOICE, { href, name })
} catch {
console.info('Unable to set WalletConnect deep link')
}
Expand Down
7 changes: 0 additions & 7 deletions packages/core/tests/controllers/AccountController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ beforeAll(() => {
describe('AccountController', () => {
it('should have valid default state', () => {
expect(AccountController.state).toEqual({
isConnected: false,
smartAccountDeployed: false,
currentTab: 0,
tokenBalance: [],
Expand All @@ -33,11 +32,6 @@ describe('AccountController', () => {
})
})

it('should update state correctly on setIsConnected()', () => {
AccountController.setIsConnected(true, chain)
expect(AccountController.state.isConnected).toEqual(true)
})

it('should update state correctly on setCaipAddress()', () => {
AccountController.setCaipAddress(caipAddress, chain)
expect(AccountController.state.caipAddress).toEqual(caipAddress)
Expand Down Expand Up @@ -81,7 +75,6 @@ describe('AccountController', () => {
it('should update state correctly on resetAccount()', () => {
AccountController.resetAccount(chain)
expect(AccountController.state).toEqual({
isConnected: false,
smartAccountDeployed: false,
currentTab: 0,
caipAddress: undefined,
Expand Down
1 change: 0 additions & 1 deletion packages/core/tests/controllers/ChainController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ describe('ChainController', () => {

it('should reset account as expected', () => {
ChainController.resetAccount(ChainController.state.activeChain)
expect(ChainController.getAccountProp('isConnected')).toEqual(false)
expect(ChainController.getAccountProp('smartAccountDeployed')).toEqual(false)
expect(ChainController.getAccountProp('currentTab')).toEqual(0)
expect(ChainController.getAccountProp('caipAddress')).toEqual(undefined)
Expand Down
19 changes: 14 additions & 5 deletions packages/core/tests/utils/StorageUtil.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('StorageUtil', () => {
const deepLink = { href: 'https://example.com', name: 'Example Wallet' }
StorageUtil.setWalletConnectDeepLink(deepLink)
const savedDL = SafeLocalStorage.getItem(SafeLocalStorageKeys.DEEPLINK_CHOICE)
expect(savedDL).toBe(deepLink.href)
expect(savedDL).toEqual({ href: deepLink.href, name: deepLink.name })
})

it('should handle errors when setting deep link', () => {
Expand All @@ -63,8 +63,14 @@ describe('StorageUtil', () => {
describe('getWalletConnectDeepLink', () => {
it('should get WalletConnect deep link from localStorage', () => {
const deepLink = { href: 'https://example.com', name: 'Example Wallet' }
SafeLocalStorage.setItem('@appkit/deeplink_choice', deepLink.href)
expect(StorageUtil.getWalletConnectDeepLink()).toEqual(deepLink.href)
SafeLocalStorage.setItem('WALLETCONNECT_DEEPLINK_CHOICE', {
href: deepLink.href,
name: deepLink.name
})
expect(StorageUtil.getWalletConnectDeepLink()).toEqual({
href: deepLink.href,
name: deepLink.name
})
})

it('should return undefined if deep link is not set', () => {
Expand All @@ -84,9 +90,12 @@ describe('StorageUtil', () => {

describe('deleteWalletConnectDeepLink', () => {
it('should delete WalletConnect deep link from localStorage', () => {
SafeLocalStorage.setItem('@appkit/deeplink_choice', 'https://example.com')
SafeLocalStorage.setItem('WALLETCONNECT_DEEPLINK_CHOICE', {
href: 'https://example.com',
name: 'Example'
})
StorageUtil.deleteWalletConnectDeepLink()
expect(SafeLocalStorage.getItem('@appkit/deeplink_choice')).toBeUndefined()
expect(SafeLocalStorage.getItem('WALLETCONNECT_DEEPLINK_CHOICE')).toBeUndefined()
})

it('should handle errors when deleting deep link', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ export class W3mSelectAddressesView extends LitElement {
}

private async onCancel() {
const { isConnected } = AccountController.state
if (isConnected) {
const { activeCaipAddress } = ChainController.state
if (activeCaipAddress) {
await ConnectionController.disconnect()
ModalController.close()
} else {
Expand Down

0 comments on commit 2a75488

Please sign in to comment.