feat: add expo-secure-store #5
Conversation
WalkthroughThe changes introduce a new module, Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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: 5
Outside diff range, codebase verification and nitpick comments (2)
vite.config.ts (1)
4-4: React version not specified in package.jsonThe
package.jsonfile does not specify a React version independencies,devDependencies, orpeerDependencies. This absence could lead to compatibility issues with the@vitejs/plugin-reactversion^4.3.1. It's recommended to explicitly declare the React version to ensure compatibility and prevent potential issues.
- Ensure that the React version is specified in
package.jsonto verify compatibility with@vitejs/plugin-react.Analysis chain
Verify the React plugin configuration and compatibility.
Ensure that the
@vitejs/plugin-reactis correctly imported and configured in the plugins array. Additionally, verify compatibility with the project's React version to prevent potential issues.Run the following script to verify the plugin's compatibility:
Also applies to: 25-25
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the project's package.json for React version and plugin compatibility. # Test: Search for React version and plugin version in package.json. Expect: Compatible versions. cat package.json | jq '.dependencies.react, .devDependencies["@vitejs/plugin-react"]'Length of output: 101
Script:
#!/bin/bash # Description: Check for React version in devDependencies and peerDependencies. # Test: Search for React version in devDependencies and peerDependencies. Expect: Version present. cat package.json | jq '.devDependencies.react, .peerDependencies.react'Length of output: 83
lib/sessionManager/types.ts (1)
13-13: Unusednonceentry inStorageKeysenum.The
nonceentry in theStorageKeysenum is currently not used in the session management code. Consider implementing its usage if it is intended for future security enhancements, or remove it if it is unnecessary to avoid confusion.
- Location:
lib/sessionManager/types.ts, line 13Analysis chain
Verify the usage and necessity of the new
nonceenum entry.Ensure that the addition of
nonceto theStorageKeysenum is necessary and correctly implemented. Verify that it is used appropriately in session management processes to enhance security, such as preventing replay attacks.Run the following script to verify the usage of
noncein the session management code:Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `nonce` in session management processes. # Test: Search for `nonce` usage in session management code. Expect: Correct implementation and usage. rg --type ts "nonce" lib/sessionManagerLength of output: 86
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
package.jsonis excluded by!**/*.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yaml
Files selected for processing (7)
- lib/main.test.ts (1 hunks)
- lib/sessionManager/index.ts (1 hunks)
- lib/sessionManager/stores/chromeStore.ts (4 hunks)
- lib/sessionManager/stores/expoSecureStore.test.ts (1 hunks)
- lib/sessionManager/stores/expoSecureStore.ts (1 hunks)
- lib/sessionManager/types.ts (1 hunks)
- vite.config.ts (2 hunks)
Files skipped from review due to trivial changes (1)
- lib/sessionManager/stores/chromeStore.ts
Additional context used
GitHub Check: main (20.x)
lib/sessionManager/stores/expoSecureStore.ts
[failure] 96-96: sessionManager/stores/expoSecureStore.test.ts > ExpoSecureStore keys: storageKeys > should set and get an item in storage: StorageKeys
TypeError: Cannot read properties of undefined (reading 'getItemAsync')
❯ ExpoSecureStore.removeSessionItem sessionManager/stores/expoSecureStore.ts:96:40
❯ ExpoSecureStore.setSessionItem sessionManager/stores/expoSecureStore.ts:46:16
❯ sessionManager/stores/expoSecureStore.test.ts:56:26
[failure] 96-96: sessionManager/stores/expoSecureStore.test.ts > ExpoSecureStore keys: storageKeys > should remove an item from storage: StorageKeys
TypeError: Cannot read properties of undefined (reading 'getItemAsync')
❯ ExpoSecureStore.removeSessionItem sessionManager/stores/expoSecureStore.ts:96:40
❯ ExpoSecureStore.setSessionItem sessionManager/stores/expoSecureStore.ts:46:16
❯ sessionManager/stores/expoSecureStore.test.ts:63:26
[failure] 96-96: sessionManager/stores/expoSecureStore.test.ts > ExpoSecureStore keys: storageKeys > should clear all items from storage: StorageKeys
TypeError: Cannot read properties of undefined (reading 'getItemAsync')
❯ ExpoSecureStore.removeSessionItem sessionManager/stores/expoSecureStore.ts:96:40
❯ ExpoSecureStore.setSessionItem sessionManager/stores/expoSecureStore.ts:46:16
❯ sessionManager/stores/expoSecureStore.test.ts:75:26
[failure] 96-96: sessionManager/stores/expoSecureStore.test.ts > ExpoSecureStore keys: storageKeys > should set and get an item in extra storage
TypeError: Cannot read properties of undefined (reading 'getItemAsync')
❯ ExpoSecureStore.removeSessionItem sessionManager/stores/expoSecureStore.ts:96:40
❯ ExpoSecureStore.setSessionItem sessionManager/stores/expoSecureStore.ts:46:16
❯ sessionManager/stores/expoSecureStore.test.ts:87:26
[failure] 96-96: sessionManager/stores/expoSecureStore.test.ts > ExpoSecureStore keys: storageKeys > should remove an item from extra storage
TypeError: Cannot read properties of undefined (reading 'getItemAsync')
❯ ExpoSecureStore.removeSessionItem sessionManager/stores/expoSecureStore.ts:96:40
❯ ExpoSecureStore.setSessionItem sessionManager/stores/expoSecureStore.ts:46:16
❯ sessionManager/stores/expoSecureStore.test.ts:94:26
[failure] 96-96: sessionManager/stores/expoSecureStore.test.ts > ExpoSecureStore keys: storageKeys > should clear all items from extra storage
TypeError: Cannot read properties of undefined (reading 'getItemAsync')
❯ ExpoSecureStore.removeSessionItem sessionManager/stores/expoSecureStore.ts:96:40
❯ ExpoSecureStore.setSessionItem sessionManager/stores/expoSecureStore.ts:46:16
❯ sessionManager/stores/expoSecureStore.test.ts:104:26
Additional comments not posted (3)
lib/sessionManager/index.ts (1)
18-18: Verify the existence and correctness of theExpoSecureStoremodule.Ensure that the file
expoSecureStore.jsexists and contains the expected module. This is crucial as the path is relative and must correctly point to the file.Run the following script to verify the file existence and content:
vite.config.ts (1)
18-20: Verify the Rollup options configuration.Ensure that the configuration to treat "react" and "react-native" as external dependencies is correct and beneficial for the build process. This configuration should optimize the build by excluding these libraries from the bundle, but it's important to verify that it does not negatively impact the application's runtime.
Run the following script to verify the build configuration and its impact:
lib/main.test.ts (1)
51-51: Approved: Addition of "ExpoSecureStore" to expected exports.The addition of "ExpoSecureStore" to the list of expected exports is correctly implemented and aligns with the PR's objectives to enhance the module's functionality.
| let expoSecureStore: typeof import("expo-secure-store") | undefined = undefined; | ||
|
|
||
| /** | ||
| * Provides a expo local store based session manager implementation for the browser. | ||
| * @class ExpoSecureStore | ||
| */ | ||
| export class ExpoSecureStore<V = StorageKeys> implements SessionManager<V> { | ||
| constructor() { | ||
| this.loadExpoStore(); | ||
| } | ||
|
|
||
| private async loadExpoStore() { | ||
| try { | ||
| expoSecureStore = await import("expo-secure-store"); | ||
| } catch (error) { | ||
| console.error("Error loading dependency expo storage:", error); | ||
| } | ||
| } |
There was a problem hiding this comment.
Issue: Handling of dynamic import for expo-secure-store.
The dynamic import of expo-secure-store is not consistently resolving, leading to expoSecureStore being undefined in some cases. This issue is evident from the static analysis failures and needs to be addressed to ensure reliable operation.
Consider initializing expoSecureStore in a more robust manner, possibly by ensuring the module is fully loaded before any session management methods are called.
Would you like me to help refactor the initialization process to ensure reliability?
DanielRivers
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5 +/- ##
===========================================
- Coverage 75.41% 62.17% -13.24%
===========================================
Files 14 15 +1
Lines 240 312 +72
Branches 36 36
===========================================
+ Hits 181 194 +13
- Misses 59 118 +59
|
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- lib/sessionManager/stores/expoSecureStore.test.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- lib/sessionManager/stores/expoSecureStore.test.ts
Explain your changes
This adds support for
expo-secure-storesession store.- dynamically imports expo-secure-store, not bundled in.
Other small changes
- Correction to JSDocs on chrome store
- Add nonce to StorageKeys
Note: Mocking in place, tests are skipped currently
Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.