Get connected account address from WalletProvider#1494
Conversation
WalkthroughThis PR introduces new wallet functionality across the application. A new Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant L as Layout Component
participant WP as WalletProvider
participant CTX as Wallet Context
participant CC as Child Component (useAccount)
U->>L: Load page
L->>WP: Initialize WalletProvider (with account prop)
WP->>CTX: Set account context (using setAccountContext)
CC->>CTX: Retrieve account (using getAccountContext)
CTX-->>CC: Return account data
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
⏰ Context from checks skipped due to timeout of 90000ms (16)
🔇 Additional comments (2)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
packages/ui-components/src/__tests__/WalletConnect.test.ts(3 hunks)packages/ui-components/src/__tests__/WalletProvider.test.ts(1 hunks)packages/ui-components/src/lib/providers/wallet/context.ts(1 hunks)packages/ui-components/src/lib/providers/wallet/useAccount.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
packages/ui-components/src/lib/providers/wallet/useAccount.ts (1)
packages/ui-components/src/lib/providers/wallet/context.ts (2) (2)
getUseAccountContext(11-14)getAccountContext(19-27)
packages/ui-components/src/lib/providers/wallet/context.ts (1)
packages/ui-components/src/lib/providers/wallet/useAccount.ts (1) (1)
useAccount(8-21)
packages/ui-components/src/__tests__/WalletConnect.test.ts (1)
packages/ui-components/src/lib/providers/wallet/useAccount.ts (1) (1)
useAccount(8-21)
🪛 Biome (1.9.4)
packages/ui-components/src/__tests__/WalletProvider.test.ts
[error] 42-42: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: git-clean
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: Deploy-Preview
- GitHub Check: test
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (10)
packages/ui-components/src/__tests__/WalletProvider.test.ts (1)
1-46: Well-structured tests for the new WalletProvider componentThe tests thoroughly verify three important behaviors of the WalletProvider:
- Setting the account store in context
- Setting the useAccount function in context
- Handling the default case when no account prop is provided
This provides good coverage for the component's core functionality.
🧰 Tools
🪛 Biome (1.9.4)
[error] 42-42: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
packages/ui-components/src/lib/providers/wallet/useAccount.ts (2)
1-21: Well-designed hook with fallback mechanismThe
useAccounthook is well-implemented with a clear fallback mechanism:
- First tries to get and use the context-provided useAccount function
- Falls back to direct account context access if needed
This provides flexibility in how account data can be accessed.
23-73: Comprehensive test coverageThe tests thoroughly verify both code paths:
- Using the useAccount function from context when available
- Falling back to direct account context when useAccount function is not available
Good job separating the test concerns and including proper mock resets in the beforeEach hook.
packages/ui-components/src/lib/providers/wallet/context.ts (3)
11-14: Good implementation of context accessorThe
getUseAccountContextfunction provides a clean way to access the useAccount function from Svelte's context.
19-27: Helpful error message in context accessorThe
getAccountContextfunction includes a helpful error message that guides developers to the likely solution when an account is not found in context.
29-82: Thorough test coverage with good edge casesThe tests cover important scenarios:
- Using useAccount function from context when available
- Falling back to direct account context
- Throwing an error when account is not in context
This ensures the hook behaves correctly under various conditions.
packages/ui-components/src/__tests__/WalletConnect.test.ts (4)
7-15: Clean transition to useAccount hookThe test has been correctly updated to use the new
useAccounthook instead of directly accessingsignerAddress. This is in line with the PR's objective of centralizing wallet state management.
33-35: Good mock implementation for disconnected stateThe mock correctly simulates the disconnected state by returning a null account.
45-47: Good mock implementation for connected stateThe mock correctly simulates the connected state by returning a valid address.
50-55: Props updated to reflect component changesThe component props have been updated to remove
signerAddress, which is now obtained through theuseAccounthook. This is consistent with the PR's objective of centralizing wallet state management.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
packages/ui-components/src/__tests__/WalletConnect.test.ts (1)
24-57: 🧹 Nitpick (assertive)Consider adding edge case tests.
The current tests cover the main scenarios (connected and disconnected), but consider adding tests for edge cases such as when the account is transitioning between states or when the wallet is connected but on an unsupported network.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
packages/ui-components/src/__tests__/DeploymentSteps.test.ts(1 hunks)packages/ui-components/src/__tests__/WalletConnect.test.ts(3 hunks)packages/ui-components/src/__tests__/WalletProvider.test.ts(1 hunks)packages/ui-components/src/lib/providers/wallet/context.ts(1 hunks)packages/ui-components/src/lib/providers/wallet/useAccount.ts(1 hunks)packages/webapp/src/lib/components/DepositOrWithdrawModal.svelte(1 hunks)packages/webapp/src/lib/components/Sidebar.svelte(2 hunks)tauri-app/src/routes/+layout.svelte(1 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
packages/ui-components/src/lib/providers/wallet/useAccount.ts (1)
packages/ui-components/src/lib/providers/wallet/context.ts (2) (2)
getUseAccountContext(10-13)getAccountContext(18-26)
packages/ui-components/src/lib/providers/wallet/context.ts (1)
packages/ui-components/src/lib/providers/wallet/useAccount.ts (1) (1)
useAccount(8-21)
packages/ui-components/src/__tests__/WalletConnect.test.ts (2)
packages/ui-components/src/lib/__mocks__/stores.ts (1) (1)
mockConnectedStore(116-121)packages/ui-components/src/lib/providers/wallet/useAccount.ts (1) (1)
useAccount(8-21)
🪛 Biome (1.9.4)
packages/ui-components/src/__tests__/WalletProvider.test.ts
[error] 46-46: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: git-clean
- GitHub Check: Deploy-Preview
- GitHub Check: test
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (16)
packages/webapp/src/lib/components/DepositOrWithdrawModal.svelte (1)
164-164: RemovedsignerAddressprop from WalletConnect componentThe removal of the
signerAddressprop from theWalletConnectcomponent aligns with the broader wallet management refactoring. This component now likely uses the context-baseduseAccounthook internally instead of receiving the address as a prop.packages/ui-components/src/__tests__/WalletProvider.test.ts (2)
1-50: Well-structured tests for the WalletProvider componentThe test cases provide good coverage of the component's functionality, including account store context setting, useAccount function context setting, and default account behavior.
🧰 Tools
🪛 Biome (1.9.4)
[error] 46-46: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
46-46: Avoid non-null assertion operatorThe non-null assertion operator (
!) bypasses TypeScript's null checks, which could lead to runtime errors if the assumption is incorrect.- const defaultAccount = accountCall![1]; + const defaultAccount = accountCall?.[1];Alternatively, add a check before accessing the value:
- const defaultAccount = accountCall![1]; + if (!accountCall) { + throw new Error('Expected setContext to be called with ACCOUNT_KEY'); + } + const defaultAccount = accountCall[1];🧰 Tools
🪛 Biome (1.9.4)
[error] 46-46: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
packages/ui-components/src/lib/providers/wallet/useAccount.ts (2)
1-21: Well-implemented useAccount hook with clear fallback strategyThe
useAccounthook is appropriately implemented with a clear strategy:
- First attempt to use the
useAccountfunction from context- Fall back to direct context access if needed
- Return a consistent object structure regardless of the path taken
The JSDoc comments also clearly indicate that this hook must be used within a child component of WalletProvider.
23-73: Comprehensive test coverageThe tests thoroughly verify both the primary and fallback behavior paths of the
useAccounthook. They confirm that:
- When the context function is available, it's properly called and its result is returned
- When the context function is not available, it correctly falls back to direct context access
This ensures the hook behaves reliably in all expected scenarios.
tauri-app/src/routes/+layout.svelte (2)
17-27: Well-designed account store derivationThe derived
accountstore elegantly combines multiple account sources with a clear priority:
- First use
ledgerWalletAddressif available- Then fall back to
walletconnectAccountif available- Return
nullif neither is setThis approach provides a single consistent source of account information throughout the application.
32-58: Appropriate integration of WalletProvider in application layoutThe
WalletProvideris correctly positioned as the outermost provider, wrapping theQueryClientProvider. This ensures that the wallet context is available throughout the component tree, including to any components that might need it within query-related components.packages/ui-components/src/lib/providers/wallet/context.ts (2)
10-13: No issues found withgetUseAccountContext.
This function is straightforward, returning data from the Svelte context as intended.
60-79: Good test coverage.
The tests thoroughly verify the presence and absence of the context function, ensuring robust functionality.packages/ui-components/src/__tests__/DeploymentSteps.test.ts (1)
12-12:Details
✅ Verification successful
Confirm correct usage of
vi.hoisted.
vi.hoistedensures mocks are registered before imports, but double-check any potential concurrency issues with dynamic imports.
🏁 Script executed:
#!/bin/bash # Locate all references to `vi.hoisted` usage. rg -A5 "vi\.hoisted"Length of output: 4936
Action: Confirm Correct
vi.hoistedUsage in DeploymentSteps TestAfter reviewing its usage across the repository, including in similar tests like OrderDetail and WalletConnect, the dynamic import via
vi.hoisted(() => import('../lib/__mocks__/stores'))inpackages/ui-components/src/__tests__/DeploymentSteps.test.tsis implemented correctly. The pattern ensures mocks are registered before module imports occur, and there’s no evidence of concurrency issues with the current usage. Please continue to monitor for any potential concurrency concerns should the dynamic import logic change in the future.packages/webapp/src/lib/components/Sidebar.svelte (2)
68-68: Reordered classes onCloseButton.
This is a minor style tweak with no functional impact. Looks fine.
107-107:Details
✅ Verification successful
Removed the
signerAddressprop.
Confirm that anything depending onsignerAddressis now handled via the context-based approach.
🏁 Script executed:
#!/bin/bash # Check for any remaining references to signerAddress in the codebase. rg "signerAddress"Length of output: 5767
Sidebar Prop Removal Verified
- In packages/webapp/src/lib/components/Sidebar.svelte the
<WalletConnect>component is now invoked without thesignerAddressprop.- The context‐based approach appears to be in place—evidenced by the use of the
signerAddressstore (e.g. in the WalletProvider in+layout.svelte) and its reactive usage in other components.- Ensure that components still accepting a
signerAddressprop (as seen in various files and tests) are intentionally handling their dependencies via the centralized store, in line with the overall migration strategy.packages/ui-components/src/__tests__/WalletConnect.test.ts (4)
7-13: Approval for the wallet provider refactoring approach.The migration from direct
signerAddressusage to the context-baseduseAccounthook is implemented correctly in these tests. The mock setup is clean and follows best practices by using Vitest's mocking capabilities.
31-33: Test correctly handles disconnected wallet state.This test setup appropriately mocks the account as
nullto simulate a disconnected wallet, matching how the actual component would behave in this scenario.
43-45: Test correctly handles connected wallet state.This test appropriately mocks the account with an Ethereum address to test the connected wallet case, and correctly verifies the truncated address display.
48-52: Props update aligns with the component interface changes.The removal of the
signerAddressprop from the render call is consistent with the refactoring goal of managing wallet connections through the context-based approach instead of passing props.
hardingjam
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/ui-components/src/__tests__/WalletProvider.test.ts (1)
1-50: 🧹 Nitpick (assertive)Suggest testing error handling
The test suite could be enhanced by adding tests for error cases or edge conditions, such as when invalid account values are provided.
Consider adding a test case like:
it('should handle invalid account values gracefully', () => { // Test with undefined, empty string, or invalid address format const invalidAccount = readable('not-an-address'); render(WalletProvider, { props: { account: invalidAccount } }); // Verify the component still functions correctly expect(vi.mocked(setContext)).toHaveBeenCalledWith(ACCOUNT_KEY, invalidAccount); expect(vi.mocked(setContext)).toHaveBeenCalledWith(USE_ACCOUNT_KEY, expect.any(Function)); });🧰 Tools
🪛 Biome (1.9.4)
[error] 45-45: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
packages/ui-components/src/__tests__/WalletProvider.test.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/ui-components/src/__tests__/WalletProvider.test.ts
[error] 45-45: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: git-clean
- GitHub Check: test
- GitHub Check: Deploy-Preview
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (5)
packages/ui-components/src/__tests__/WalletProvider.test.ts (5)
1-9: Good test setup and imports.The import statements are well-organized and appropriately import necessary testing utilities, the component being tested, and Svelte utilities.
10-13: Well-structured mock for Svelte context functions.Good practice mocking the Svelte
setContextandgetContextfunctions to isolate tests from actual context implementation.
15-26: Good test for account store context setting.This test clearly verifies that the component correctly sets the account store in the Svelte context using the appropriate key.
28-38: Appropriate verification of useAccount function.The test correctly verifies that the useAccount function is set in context, using
expect.any(Function)to check the type without being too strict about the implementation.
40-48: Avoid non-null assertion operatorThe non-null assertion operator (
!) bypasses TypeScript's null checks, which could lead to runtime errors if the assumption is incorrect.-const defaultAccount = accountCall![1]; +const defaultAccount = accountCall?.[1];Alternatively, add a check before accessing the value:
-const defaultAccount = accountCall![1]; +if (!accountCall) { + throw new Error('Expected setContext to be called with ACCOUNT_KEY'); +} +const defaultAccount = accountCall[1];🧰 Tools
🪛 Biome (1.9.4)
[error] 45-45: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
packages/ui-components/src/__tests__/WalletProvider.test.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/ui-components/src/__tests__/WalletProvider.test.ts
[error] 45-45: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: Deploy-Preview
- GitHub Check: test
- GitHub Check: test
- GitHub Check: git-clean
- GitHub Check: test
🔇 Additional comments (4)
packages/ui-components/src/__tests__/WalletProvider.test.ts (4)
1-14: Good test setup with proper mocking.The test setup is well-structured with appropriate imports and mocking of the
setContextandgetContextfunctions, which are essential for testing the context-based functionality of the WalletProvider component.
15-26: Well-implemented test for account context setting.The test correctly verifies that the account store is properly set in the context when provided as a prop to the WalletProvider component.
28-38: Good verification of the useAccount function in context.This test appropriately checks that the useAccount function is set in the context, which is a critical part of the WalletProvider's functionality.
40-48: Avoid non-null assertion operator and enhance the default account test.The non-null assertion operator (
!) in line 45 bypasses TypeScript's null checks, which could lead to runtime errors ifaccountCallis undefined.- const defaultAccount = accountCall![1]; + const defaultAccount = accountCall?.[1];Also, the test could be more comprehensive by verifying the properties of the default account and checking that the useAccount function is set in context:
it('should use default null account when no account prop provided', () => { render(WalletProvider); const setContextCalls = vi.mocked(setContext).mock.calls; const accountCall = setContextCalls.find((call) => call[0] === ACCOUNT_KEY); - const defaultAccount = accountCall![1]; + const defaultAccount = accountCall?.[1]; + const useAccountCall = setContextCalls.find((call) => call[0] === USE_ACCOUNT_KEY); expect(defaultAccount).toBeDefined(); + // Verify the default account store has the expected behavior + expect(defaultAccount.subscribe).toBeDefined(); + + // Verify useAccount function is set + expect(useAccountCall?.[1]).toEqual(expect.any(Function)); });🧰 Tools
🪛 Biome (1.9.4)
[error] 45-45: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tauri-app/vite.config.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: git-clean
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
- GitHub Check: test
- GitHub Check: Deploy-Preview
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (3)
tauri-app/vite.config.ts (3)
54-70: Updated test configuration enhances test setup for new wallet functionalityThe test configuration has been properly updated to support testing of the new WalletProvider component. The changes include:
- Added Jest-like globals for improved test compatibility
- Specified test file inclusion patterns
- Added environment variables loading
- Set appropriate test timeout
- Updated server dependencies to inline only @tanstack/svelte-query
These changes will facilitate effective testing of the new wallet functionality being introduced in this PR.
56-56:Details
✅ Verification successful
Verify the purpose of
includeSourceThe
includeSourceproperty appears to be used for specifying source files, but this may not be a standard Vitest property. Confirm if this should becoverage.includeinstead, which is commonly used to specify which files to include in code coverage reporting.
🌐 Web query:
Is includeSource a valid configuration property in Vitest?💡 Result:
Yes,
includeSourceis a valid configuration property in Vitest. It allows you to run in-source tests directly within your codebase by specifying glob patterns for files containing test blocks defined withimport.meta.vitest[5][19].Key Details about
includeSource
- Purpose: Detects test blocks embedded in source files (not just dedicated test files like
*.test.ts).- Configuration:
// vitest.config.ts import { defineConfig } from 'vitest/config' export default defineConfig({ test: { includeSource: ['src/**/*.{js,ts}'] // Checks .js/.ts files in src/ } })- Production Build: Requires
import.meta.vitestto be set toundefinedto exclude tests from bundles:define: { 'import.meta.vitest': 'undefined' }Example Usage
In a source file (
math.ts):export function add(a: number, b: number) { return a + b } // In-source test if (import.meta.vitest) { const { test, expect } = import.meta.vitest test('adds numbers', () => { expect(add(2, 3)).toBe(5) }) }This configuration enables Rust-style in-source testing, where tests coexist with implementation code[19].
Citations:
- 1: https://vitest.dev/guide/
- 2: https://github.com/vitest-dev/vitest/blob/main/docs/config/index.md?plain=1
- 3: https://app.studyraid.com/en/read/11292/352297/configuring-vitest-options
- 4: https://www.thecandidstartup.org/2024/08/19/vitest-monorepo-setup.html
- 5: https://vitest.dev/config/
- 6: https://allurereport.org/docs/vitest-configuration/
- 7: https://vitest.dev/guide/browser/multiple-setups
- 8: wallabyjs/public#3108
- 9: https://aronschueler.de/blog/2024/09/26/In-Source-testing-with-vite-and-vitest/
- 10: https://vitest.dev/guide/browser/config
- 11: https://vitest.dev/guide/cli
- 12: nuxt/test-utils#872
- 13: https://joranquinten.nl/articles/in-source-tests-with-vitest
- 14: https://vitest.dev/guide/workspace
- 15: vitest-dev/vitest#5121
- 16: https://v0.vitest.dev/config/
- 17: vitest-dev/vitest#4186
- 18: https://semaphoreci.com/blog/vitest
- 19: https://vitest.dev/guide/in-source
- 20: https://v1.vitest.dev/config/
Action: Confirm Correct Use of
includeSourcein Vitest ConfigurationThe web query confirms that
includeSourceis a valid Vitest property and is intended for detecting in-source test blocks (usingimport.meta.vitest). It is not a mistake in configuration, and there is no need to replace it withcoverage.include.
67-67:Details
❓ Verification inconclusive
Confirm if removing @sveltejs/kit from inline dependencies is intentional
The configuration now only inlines @tanstack/svelte-query, whereas it appears @sveltejs/kit might have been inlined previously. Ensure this change was intentional and won't affect the test environment's ability to properly handle SvelteKit imports.
🏁 Script executed:
#!/bin/bash # Check for related dependencies in package.json and look for other uses of @sveltejs/kit echo "Checking package.json for related dependencies..." cat package.json | grep -A 10 -B 10 "@sveltejs/kit" || echo "No direct references found in package.json" echo -e "\nChecking for other imports of @sveltejs/kit in source files..." rg "from '@sveltejs/kit" --type tsLength of output: 1507
Confirm the Removal of @sveltejs/kit from Inline Dependencies
Our investigation confirms that while the package still exists (as seen in package.json) and is imported in several parts of the codebase (e.g., in multiple vite.config.ts files and SvelteKit-related modules), the inline configuration in
tauri-app/vite.config.tsnow only inlines@tanstack/svelte-query. Please verify that the removal of@sveltejs/kitfrom the inline list was a deliberate change and that it won’t adversely affect Vite’s handling of SvelteKit modules during development and testing.
- File Affected:
tauri-app/vite.config.ts(line 67)- Observations:
- The inline configuration now only includes
[@tanstack/svelte-query].- Other parts of the repository are still importing
@sveltejs/kit(e.g., inpackages/webapp/vite.config.ts,packages/ui-components/vite.config.ts, etc.).Please confirm this change is intentional and that any potential impacts on module optimization or test environment behavior have been accounted for.
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
packages/ui-components/src/lib/providers/wallet/context.ts (1)
27-63: 🧹 Nitpick (assertive)Extend test coverage to
setAccountContext.Although the tests thoroughly cover
getAccountContext, there is no unit test forsetAccountContext. Consider adding the following snippet (or something similar) to ensure future refactors do not inadvertently break its functionality:+ it('should set the account in context', () => { + const mockAccount = readable('0x456'); + setAccountContext(mockAccount); + expect(setContext).toHaveBeenCalledWith(ACCOUNT_KEY, mockAccount); + });tauri-app/src/lib/stores/walletconnect.ts (1)
40-42:⚠️ Potential issueUpdate store setter calls to match the new type.
The
walletconnectAccountstore is now typed asHex | null, but it's still being set toundefinedin these locations. This creates a type mismatch that could lead to runtime issues.Apply these changes to ensure type consistency with the new store definition:
- walletconnectAccount.set(provider?.accounts?.[0] ?? undefined); + walletconnectAccount.set(provider?.accounts?.[0] as Hex ?? null); - walletconnectAccount.set(undefined); + walletconnectAccount.set(null); - walletconnectAccount.set(accounts?.[0] ?? undefined); + walletconnectAccount.set(accounts?.[0] as Hex ?? null); - walletconnectAccount.set(undefined); + walletconnectAccount.set(null);This ensures the values being set match the defined type and prevents potential type errors.
Also applies to: 43-45, 46-48, 106-106
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
packages/ui-components/src/__tests__/WalletProvider.test.ts(1 hunks)packages/ui-components/src/lib/providers/wallet/WalletProvider.svelte(1 hunks)packages/ui-components/src/lib/providers/wallet/context.ts(1 hunks)packages/ui-components/src/lib/providers/wallet/useAccount.ts(1 hunks)packages/ui-components/src/lib/types/account.ts(1 hunks)packages/webapp/src/lib/stores/wagmi.ts(1 hunks)tauri-app/src/lib/stores/walletconnect.ts(2 hunks)tauri-app/src/lib/stores/wallets.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
packages/ui-components/src/__tests__/WalletProvider.test.ts (2)
packages/ui-components/src/lib/types/account.ts (1)
Account(4-4)packages/ui-components/src/lib/providers/wallet/context.ts (1)
setAccountContext(23-25)
packages/ui-components/src/lib/providers/wallet/useAccount.ts (1)
packages/ui-components/src/lib/providers/wallet/context.ts (1)
getAccountContext(10-18)
packages/ui-components/src/lib/providers/wallet/context.ts (1)
packages/ui-components/src/lib/types/account.ts (1)
Account(4-4)
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
- GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: test
- GitHub Check: test
- GitHub Check: test
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: git-clean
- GitHub Check: Deploy-Preview
🔇 Additional comments (10)
packages/ui-components/src/lib/types/account.ts (1)
1-4: Clean type definition that provides good flexibilityThe
Accounttype is well-defined as a Svelte store that can contain either a hexadecimal address or null, making it perfect for representing both connected and disconnected wallet states.packages/ui-components/src/lib/providers/wallet/WalletProvider.svelte (1)
1-11: LGTM: Clean implementation of a context provider componentThe
WalletProvidercomponent follows a good pattern for providing context in Svelte applications. It properly sets a default value for the account (a readable store with null), making it easy to represent an unconnected state when no account is provided.packages/ui-components/src/__tests__/WalletProvider.test.ts (1)
1-29: Good test implementation with proper mockingThe test correctly verifies that the
WalletProvidercomponent callssetAccountContextwith the provided account prop. The mocking approach is clean, and includingbeforeEachto clear mocks between tests is a good practice.packages/ui-components/src/lib/providers/wallet/useAccount.ts (1)
16-42: Well-structured inline testsThe inline test implementation is clean and thorough, properly mocking dependencies and verifying the expected behavior of the hook.
tauri-app/src/lib/stores/wallets.ts (1)
4-6: Use ofHex | nullimproves type safety.Switching from
string | undefinedtoHex | nullis a strong move toward stricter typing for wallet addresses. This change aligns well with the approach used in other wallet-related stores.packages/webapp/src/lib/stores/wagmi.ts (1)
17-22: Consistent adoption ofHex | null.Adopting
Hex | nullforsignerAddressharmonizes type usage across the codebase. This improvement also helps reduce potential confusion by explicitly conveying that the address may be null if disconnected.packages/ui-components/src/lib/providers/wallet/context.ts (2)
1-6: Dedicated context key setup looks good.Defining
ACCOUNT_KEYhere clarifies context usage, facilitating maintainability and clear separation of concerns.
7-18: Well-documented context retrieval.The
getAccountContextfunction provides a straightforward method for retrieving the account store, with a clear and helpful error message prompting proper usage.tauri-app/src/lib/stores/walletconnect.ts (2)
8-8: Appropriate type import added for Hex addresses.Adding the
Hextype from 'viem' is a good improvement for type safety, ensuring wallet addresses are properly typed. The imports forhexToNumberandisHexare also being used at line 50, keeping imports consistent with usage.
49-52: Existing code is already using the imported viem functions.This section correctly uses the
isHexandhexToNumberfunctions from viem. The conditional check ensures that chain IDs are properly converted to numbers regardless of their format (hex or decimal).🧰 Tools
🪛 Biome (1.9.4)
[error] 51-51: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.(lint/style/useNumberNamespace)
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
packages/ui-components/src/__tests__/WalletProvider.test.ts(1 hunks)packages/ui-components/src/lib/index.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
packages/ui-components/src/__tests__/WalletProvider.test.ts (2)
packages/ui-components/src/lib/types/account.ts (1)
Account(4-4)packages/ui-components/src/lib/providers/wallet/context.ts (1)
setAccountContext(23-25)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: test
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: git-clean
- GitHub Check: Deploy-Preview
🔇 Additional comments (5)
packages/ui-components/src/lib/index.ts (2)
133-133: LGTM: New WalletProvider export addedThe addition of the WalletProvider export aligns with the PR's objective of implementing a wrapper to expose connected account information to descendant components. The export is correctly placed in the "Providers" section of the file.
135-137: LGTM: Hook section organized with new useAccount exportGood organization of the "Hooks" section following the "Providers" section. The new useAccount hook export complements the WalletProvider by providing a way for descendant components to access the connected account information from the Svelte context.
packages/ui-components/src/__tests__/WalletProvider.test.ts (3)
1-12: Import structure and mock setup look goodThe imports and mock setup are clean and well-organized. You're properly using vitest's mocking capabilities to isolate the component under test.
13-16: Good practice: Using beforeEach to reset mocksUsing
beforeEachto clear all mocks before each test is a good practice that helps prevent test interference.
30-42: Well-implemented test for the default caseThis test thoroughly checks the default behavior when no account is provided. It properly extracts and verifies the value from the store.
2 participants
Motivation
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
WalletProvidercomponent facilitates wallet functionality integration.useAccounthook for accessing wallet account information.Bug Fixes
data-testidattribute for testing in theVaultDetailcomponent.Tests
WalletProvidercomponent to validate account context handling.