feat: prime login and payment #6480
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
WalkthroughThe pull request introduces extensive changes across various project components. New environment variables and a shared environment setup replace older dotenv actions in GitHub workflows. Several workflow files, build scripts, and configuration files are updated to enhance setup, disk space management, and build processes. Sentry integration is improved by adding a fallback error boundary parameter. Major updates include new UI components, hooks, routers, state atoms, and services to support OneKey Prime features like login, subscription management, and device handling. Additional utility, error, and request/header management functions are refined, and new modules are added for Google Play services. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant D as PrimeLoginEmailDialog
participant SP as ServicePrime API
participant P as PrimePersistAtom
U->>D: Enter email and submit
D->>SP: Call apiSendEmailVerificationCode
SP-->>D: Return verification code response
U->>D: Enter verification code and confirm
D->>SP: Call apiPrimeLogin with code
SP-->>D: Return login response
SP->>P: Update persistent login state
D-->>U: Display success message
✨ 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 (
|
sidmorizon
force-pushed
the
feat/prime-login0
branch
from
3f5e092 to
cbafd0f
Compare
|
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
sidmorizon
force-pushed
the
feat/prime-login0
branch
3 times, most recently
from
2ed7a9b to
358a2a3
Compare
sidmorizon
force-pushed
the
feat/prime-login0
branch
6 times, most recently
from
4d7ea5b to
b7a34cb
Compare
sidmorizon
force-pushed
the
feat/prime-login0
branch
2 times, most recently
from
9a467a4 to
cafdfae
Compare
sidmorizon
force-pushed
the
feat/prime-login0
branch
from
cafdfae to
a1d3d51
Compare
* feat: update confirmButtonProps * feat: refactor Prime dashboard Lottie animation rendering * refactor: simplify Prime authentication hooks and remove unused code * refactor: optimize Prime banner background dark mode Lottie animation * feat: update app * refactor: adjust Prime Lottie animation rendering for web and native platforms --------- Co-authored-by: limichange <[email protected]> Co-authored-by: morizon <[email protected]>
…6618) * refactor: simplify Privy authentication configuration and login flow * fix: improve Prime login email code dialog error handling * feat: add environment variable support for Privy authentication
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
🛑 Comments failed to post (159)
packages/kit-bg/src/services/ServiceNotification/PushProvider/PushProviderWebSocket.ts (1)
33-37: 🧹 Nitpick (assertive)
Document the timeout value.
Good use of typed event names and timeout handling. Consider adding a comment explaining why 3000ms was chosen as the timeout value.
packages/kit/src/views/Prime/pages/PrimeDashboard/PrimeUserInfoMoreButton.tsx (4)
115-115: 🛠️ Refactor suggestion
Use translation for dialog description.
The dialog description is hardcoded in English. Use the translation system for consistency.
- description: 'Are you sure you want to log out?', + description: intl.formatMessage({ + id: ETranslations.prime_logout_confirmation, + }),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.description: intl.formatMessage({ id: ETranslations.prime_logout_confirmation, }),
158-160: 🛠️ Refactor suggestion
Remove console.log statement.
Remove debugging console.log statement from production code.
- onPress={() => { - console.log('1'); - }} + onPress={() => {}}
133-138: 🧹 Nitpick (assertive)
Remove unused parameter.
The
handleActionListOpenparameter is declared but never used.({ handleActionListClose, - }: { - handleActionListClose: () => void; - handleActionListOpen: () => void; - }) + }: { + handleActionListClose: () => void; + })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.({ handleActionListClose, }: { handleActionListClose: () => void; }) => (
64-74: 🧹 Nitpick (assertive)
Remove commented-out code.
Delete unused code instead of commenting it out. If you need it later, you can find it in version control.
- {/* <ActionList.Item - label="Change email" - icon="EmailOutline" - onClose={handleActionListClose} - onPress={() => { - Toast.success({ - title: 'Change email not implemented', - }); - updateEmail(); - }} - /> */}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.packages/kit/src/views/Developer/pages/Gallery/Components/stories/WebEmbed.tsx (4)
66-66: 🧹 Nitpick (assertive)
Remove commented code
Delete the commented WebViewWebEmbed component if it's no longer needed.
57-57: 🧹 Nitpick (assertive)
Move localhost URL to configuration
Hardcoded localhost URL should be in a configuration file for better environment management.
- setUrl('http://localhost:3008'); + setUrl(process.env.DEFAULT_WEBVIEW_URL || 'http://localhost:3008');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.setUrl(process.env.DEFAULT_WEBVIEW_URL || 'http://localhost:3008');
68-71: 🛠️ Refactor suggestion
Add error handling to RPC test
The RPC test call lacks error handling. Add try-catch to handle potential failures gracefully.
- const result = await webembedApiProxy.test.test1('a', 'b', 'c', 'd'); - alert(JSON.stringify(result)); + try { + const result = await webembedApiProxy.test.test1('a', 'b', 'c', 'd'); + alert(JSON.stringify(result)); + } catch (error) { + alert(`RPC test failed: ${error.message}`); + }
82-84: 🧹 Nitpick (assertive)
Add meaningful content to Layout props
The description and arrays are empty. Add useful content or remove if unnecessary.
- description="..." - suggestions={[]} - boundaryConditions={[]} + description="Web embed configuration interface for development purposes" + suggestions={['Use local network IP for real devices', 'Update white list in environment variables']} + boundaryConditions={['Requires local development server running on port 3008']}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.description="Web embed configuration interface for development purposes" suggestions={['Use local network IP for real devices', 'Update white list in environment variables']} boundaryConditions={['Requires local development server running on port 3008']}packages/kit/src/views/Prime/components/PrimeLoginEmailDialogV2/PrimeLoginEmailDialogV2.tsx (4)
84-86: 🛠️ Refactor suggestion
Internationalize validation messages.
Hardcoded English messages should use intl.formatMessage.
if (!value) { - return 'email is required'; + return intl.formatMessage({ id: ETranslations.form_email_required }); } ... required: { value: true, - message: 'email is required', + message: intl.formatMessage({ id: ETranslations.form_email_required }), },Also applies to: 93-95
107-107: 🧹 Nitpick (assertive)
Improve input text trimming.
The trim logic is unclear and could be simplified.
- onChangeText={(text) => text?.trim() ?? text} + onChangeText={(text) => text?.trim() || ''}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.onChangeText={(text) => text?.trim() || ''}
57-60:
⚠️ Potential issueImprove error handling in submit function.
The catch block swallows errors after calling preventClose.
} catch (error) { options?.preventClose?.(); - throw error; + Dialog.show({ + title: 'Error', + description: 'Failed to send verification code. Please try again.', + }); + console.error('Email submission failed:', error); }
23-25:
⚠️ Potential issueImprove error handling.
Don't just log errors to console. Show error messages to users.
onError: (error) => { - console.log(error); + Dialog.show({ + title: 'Error', + description: error.message || 'Failed to login. Please try again.', + }); },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.onError: (error) => { Dialog.show({ title: 'Error', description: error.message || 'Failed to login. Please try again.', }); },packages/kit/src/views/Prime/pages/PrimeDashboard/PrimeSubscriptionPlans.tsx (5)
122-122:
⚠️ Potential issueRemove unsafe type casting.
The double type casting (
as unknown as any) is dangerous and bypasses TypeScript's type checking.- p.rcBillingProduct?.normalPeriodDuration as unknown as any + p.rcBillingProduct?.normalPeriodDuration as PeriodDuration
128-128: 🛠️ Refactor suggestion
Move currency to configuration.
Don't hardcode the currency symbol. Use a configuration value or pass it as a prop.
- currency="$" + currency={config.defaultCurrency}
35-40: 🛠️ Refactor suggestion
Remove commented code and implement dynamic savings calculation.
Dead code adds noise. Either implement the percentage calculation or remove it entirely.
- // const savePercent = new BigNumber(1) - // .minus(pricePerMonthBN.div(price)) - // .multipliedBy(100) - // .toFixed(1); - // promoText = `Save ${savePercent}%`; - promoText = `Save 33%`; + const savePercent = new BigNumber(1) + .minus(pricePerMonthBN.div(price)) + .multipliedBy(100) + .toFixed(1); + promoText = `Save ${savePercent}%`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const savePercent = new BigNumber(1) .minus(pricePerMonthBN.div(price)) .multipliedBy(100) .toFixed(1); promoText = `Save ${savePercent}%`;
81-81: 💡 Verification agent
🧩 Analysis chain
Address the i18n TODO comment.
Implement internationalization for the '/month' text to support multiple languages.
🏁 Script executed:
#!/bin/bash # Check if i18n is already set up in the project rg -l "i18n" --type tsLength of output: 1262
Internationalize the '/month' string in PrimeSubscriptionPlans.tsx
The code still uses the hard-coded text. Please update it to use your i18n function (for example, something like i18n.t('subscriptionPlans.perMonth')) and add the key to your translation files.
• File: packages/kit/src/views/Prime/pages/PrimeDashboard/PrimeSubscriptionPlans.tsx, line 81
26-26: 🧹 Nitpick (assertive)
Use a more type-safe approach for period duration.
Replace the string literal type with an enum to prevent typos and improve maintainability.
- periodDuration: 'P1Y' | 'P1M'; +enum PeriodDuration { + YEARLY = 'P1Y', + MONTHLY = 'P1M' +} + periodDuration: PeriodDuration;Committable suggestion skipped: line range outside the PR's diff.
packages/kit-bg/src/providers/ProviderApiPrivate.ts (1)
511-516: 🧹 Nitpick (assertive)
Enhanced logging improves debugging.
The switch to
JSON.stringifymakes the log output more structured and readable.Consider adding error handling around
JSON.stringifyto handle circular references:- logName: `ProviderApiPrivate.callWebEmbedApiProxy: ${JSON.stringify({ + logName: `ProviderApiPrivate.callWebEmbedApiProxy: ${JSON.stringify({ module: data?.module, method: data?.method, isWebEmbedApiReady: Boolean(this.isWebEmbedApiReady), webEmbedBridge: Boolean(bg?.webEmbedBridge), - })}`, + }, null, 2)}`.replace(/\n\s*/g, ' '),apps/web-embed/index.js (1)
16-34: 🧹 Nitpick (assertive)
Consider a more descriptive fallback.
The Suspense fallback is just an empty div. A simple loading indicator might be friendlier. Everything else looks straightforward.
packages/kit/src/views/Prime/purchasesSdk/PurchasesSdkWebBase.ts (4)
14-28: 🧹 Nitpick (assertive)
Use consistent error handling
You're throwing an error if userId is missing. That's good. However, consider standardizing error codes or messages across the codebase so logs remain consistent.
36-41: 🧹 Nitpick (assertive)
Avoid logging sensitive info
Consider removing or redacting user details in console logs to protect privacy in production.
43-64: 🧹 Nitpick (assertive)
Future-proof your package sorting
Right now, you're returning 'Yearly' first. If you add more offering types, a single check might be restrictive. A flexible approach to sorting can help.
66-99: 🛠️ Refactor suggestion
Enhance purchase error handling
You're throwing a generic error for invalid package IDs. Use more descriptive messages for easier debugging. Also handle cases where the user already purchased.packages/kit/src/views/Prime/hooks/usePrimePayment.native.ts (2)
28-71: 🧹 Nitpick (assertive)
Add user logout logic if needed
You log in with a user ID, but the logout code is commented out. If the user changes, a logout step might be needed.
129-185: 🧹 Nitpick (assertive)
Handle alternative store flows
You handle Google Play. Consider a fallback or more explicit handling for other stores.packages/kit/src/views/Prime/hooks/usePrimePayment.ts (2)
34-103: 🧹 Nitpick (assertive)
Consider race conditions
You configure Purchases inside getCustomerInfo(). Repeated calls might trigger multiple inits. Guard or cache as needed.
154-199: 🧹 Nitpick (assertive)
Unify error messages
A single "Invalid packageId" error may be too broad. Provide a unique code or message.packages/kit/src/components/WebViewWebEmbed/index.tsx (1)
47-60: 🧹 Nitpick (assertive)
Handle missing RevenueCat key gracefully.
Throwing an error may cause unhandled promise rejections if unmounted. Consider try/catch or user feedback for missing keys.packages/kit-bg/src/services/ServicePrime/ServicePrime.tsx (4)
46-60: 🧹 Nitpick (assertive)
getPrimeClient interceptor.
It injects the auth token if present. This is a clean approach. Consider logging or user feedback if token is unavailable.
399-414: 🛠️ Refactor suggestion
promptPrimeLoginEmailDialog with async promise executor (lines 402-411).
Avoid using async in a Promise executor to prevent unexpected rejections.Apply this patch:
- const email = await new Promise<string>(async (resolve, reject) => { + const email = await new Promise<string>((resolve, reject) => {Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 402-411: Promise executor functions should not be
async.(lint/suspicious/noAsyncPromiseExecutor)
440-465: 🛠️ Refactor suggestion
promptPrimeLoginPasswordDialog with async promise executor (lines 449-462).
Avoid using async in a Promise executor.Apply this patch:
- const password = await new Promise<string>(async (resolve, reject) => { + const password = await new Promise<string>((resolve, reject) => {Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 449-462: Promise executor functions should not be
async.(lint/suspicious/noAsyncPromiseExecutor)
488-512: 🛠️ Refactor suggestion
promptPrimeLoginEmailCodeDialog with async promise executor (lines 497-510).
Avoid using async in the executor.Apply:
- const code = await new Promise<string>(async (resolve, reject) => { + const code = await new Promise<string>((resolve, reject) => {Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 497-510: Promise executor functions should not be
async.(lint/suspicious/noAsyncPromiseExecutor)
packages/shared/src/googlePlayService/googlePlayServiceTypes.ts (1)
1-3: 🧹 Nitpick (assertive)
Clean interface design!
The interface is well-defined and focused. Consider adding JSDoc comments to document the purpose and behavior of the
isAvailablemethod.export type IGooglePlayService = { + /** Checks if Google Play Services is available on the device */ isAvailable: () => Promise<boolean>; };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export type IGooglePlayService = { /** Checks if Google Play Services is available on the device */ isAvailable: () => Promise<boolean>; };packages/shared/src/consts/webEmbedConsts.ts (1)
1-5: 🧹 Nitpick (assertive)
Well-structured route constants!
The enum follows good naming patterns and uses clear, RESTful paths. Consider adding JSDoc comments to document each route's purpose.
export enum EWebEmbedRoutePath { + /** Home page route */ index = '/', + /** Prime subscription purchase page */ primePurchase = '/prime/purchase', + /** Web embed API endpoint */ webEmbedApi = '/webembed/api', }apps/desktop/webpack.config.js (1)
1-1: 💡 Verification agent
🧩 Analysis chain
Verify environment setup.
The environment setup is now required before webpack config. Ensure all necessary environment variables are defined in the
.envfile.
🏁 Script executed:
#!/bin/bash # Check if required env file exists and contains necessary variables if [ -f "../../.env" ]; then echo "Checking .env file..." grep -E "^(ENABLE_ANALYZER|PRIVY_APP_ID|REVENUECAT_API_KEY)=" ../../.env || echo "Missing required environment variables" else echo ".env file not found" fiLength of output: 72
Environment setup issue detected.
Our check shows the required .env file is missing. The script did not find the file at "../../.env".
Please ensure a .env file exists with the necessary variables, such as ENABLE_ANALYZER, PRIVY_APP_ID, and REVENUECAT_API_KEY.packages/shared/src/googlePlayService/googlePlayService.ts (1)
3-5:
⚠️ Potential issueImplement actual Google Play services check
The current implementation always returns
false. This could cause issues on Android devices where Google Play services are actually available.Consider using the
@react-native-community/google-signinpackage for proper detection:-const googlePlayService: IGooglePlayService = { - isAvailable: () => Promise.resolve(false), -}; +import { GoogleSignin } from '@react-native-community/google-signin'; + +const googlePlayService: IGooglePlayService = { + isAvailable: async () => { + try { + return await GoogleSignin.hasPlayServices(); + } catch { + return false; + } + }, +};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.import { GoogleSignin } from '@react-native-community/google-signin'; const googlePlayService: IGooglePlayService = { isAvailable: async () => { try { return await GoogleSignin.hasPlayServices(); } catch { return false; } }, };packages/shared/src/routes/prime.ts (2)
1-4: 🧹 Nitpick (assertive)
Add documentation for routes
Add JSDoc comments to describe each route's purpose and usage.
Example:
export enum EPrimePages { + /** Main dashboard for Prime features */ PrimeDashboard = 'PrimeDashboard', + /** Device limit warning/management screen */ PrimeDeviceLimit = 'PrimeDeviceLimit', }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export enum EPrimePages { /** Main dashboard for Prime features */ PrimeDashboard = 'PrimeDashboard', /** Device limit warning/management screen */ PrimeDeviceLimit = 'PrimeDeviceLimit', }
6-9: 🧹 Nitpick (assertive)
Consider consistent naming pattern
The route names mix different patterns:
PrimevsPrimeDevice. Consider using consistent prefixing.Example:
export type IPrimeParamList = { [EPrimePages.PrimeDashboard]: undefined; - [EPrimePages.PrimeDeviceLimit]: undefined; + [EPrimePages.PrimeDevicesLimit]: undefined; };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export type IPrimeParamList = { [EPrimePages.PrimeDashboard]: undefined; [EPrimePages.PrimeDevicesLimit]: undefined; };apps/web-embed/webpack.config.js (1)
5-8:
⚠️ Potential issueRemove sensitive information from console logs.
The RevenueCat API key is sensitive. Remove this console.log to prevent exposing it in browser consoles.
-console.log( - 'process.env.REVENUECAT_API_KEY_WEB_SANDBOX', - process.env.REVENUECAT_API_KEY_WEB_SANDBOX, -);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.apps/web-embed/utils/webEmbedAppSettings.ts (2)
12-14: 🧹 Nitpick (assertive)
Consider using named exports.
Default exports can make code harder to refactor. Consider using named exports instead.
-export default { - getSettings, -}; +export { getSettings };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export { getSettings };
7-10: 🧹 Nitpick (assertive)
Add type safety checks.
The settings getter could be more robust. Add type checking and default values.
function getSettings(): IWebEmbedOnekeyAppSettings | undefined { - const settings = globalThis.WEB_EMBED_ONEKEY_APP_SETTINGS; - return settings; + const settings = globalThis.WEB_EMBED_ONEKEY_APP_SETTINGS; + if (!settings) return undefined; + + return { + themeVariant: settings.themeVariant || 'default', + localeVariant: settings.localeVariant || 'en', + revenuecatApiKey: settings.revenuecatApiKey + }; }packages/kit/src/views/Prime/purchasesSdk/PurchasesSdkWebEmbed.ts (1)
4-12: 🛠️ Refactor suggestion
Add type safety and improve error handling
The
getApiKeymethod needs improvements:
- Empty string fallback is risky
- Missing type assertion for settings
- Environment check could be more explicit
Here's a safer implementation:
async getApiKey(): Promise<string> { const settings = globalThis.WEB_EMBED_ONEKEY_APP_SETTINGS; - if (process.env.NODE_ENV !== 'production') { + const isDevelopment = process.env.NODE_ENV === 'development'; + if (isDevelopment) { if (!settings?.revenuecatApiKey) { - return process.env.REVENUECAT_API_KEY_WEB_SANDBOX || ''; + const sandboxKey = process.env.REVENUECAT_API_KEY_WEB_SANDBOX; + if (!sandboxKey) { + throw new Error('Sandbox API key not found'); + } + return sandboxKey; } } - return settings?.revenuecatApiKey || ''; + const apiKey = settings?.revenuecatApiKey; + if (!apiKey) { + throw new Error('API key not found'); + } + return apiKey; }apps/web-embed/pages/PageIndex.tsx (1)
7-17: 🧹 Nitpick (assertive)
Improve component structure and styling
Move inline styles to a CSS module and refactor navigation links.
Here's a cleaner implementation:
+ const NAV_ITEMS = [ + { path: EWebEmbedRoutePath.webEmbedApi, label: 'WebEmbedApi' }, + { path: EWebEmbedRoutePath.primePurchase, label: 'PrimePurchase' }, + ]; return ( - <div style={{ padding: '20px' }}> + <div className={styles.container}> <h1>PageIndex</h1> <ul> - <li> - <Link to={EWebEmbedRoutePath.webEmbedApi}>WebEmbedApi</Link> - </li> - <li> - <Link to={EWebEmbedRoutePath.primePurchase}>PrimePurchase</Link> - </li> + {NAV_ITEMS.map(({ path, label }) => ( + <li key={path}> + <Link to={path}>{label}</Link> + </li> + ))} </ul> </div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const NAV_ITEMS = [ { path: EWebEmbedRoutePath.webEmbedApi, label: 'WebEmbedApi' }, { path: EWebEmbedRoutePath.primePurchase, label: 'PrimePurchase' }, ]; return ( <div className={styles.container}> <h1>PageIndex</h1> <ul> {NAV_ITEMS.map(({ path, label }) => ( <li key={path}> <Link to={path}>{label}</Link> </li> ))} </ul> </div> );apps/desktop/App.tsx (1)
15-16: 🧹 Nitpick (assertive)
Remove commented code
Delete unused commented export.
export default withSentryHOC(KitProvider, SentryErrorBoundaryFallback); - // export default KitProvider;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export default withSentryHOC(KitProvider, SentryErrorBoundaryFallback);apps/web-embed/pages/WebEmbedAppProvider.tsx (1)
15-16: 🛠️ Refactor suggestion
Replace type casting with proper types
Remove 'any' type casting and use proper types for theme and locale.
- theme={(settings?.themeVariant as any) || 'light'} - locale={(settings?.localeVariant as any) || 'en-US'} + theme={settings?.themeVariant || 'light'} + locale={settings?.localeVariant || 'en-US'}Add type definitions:
type ThemeVariant = 'light' | 'dark'; type LocaleVariant = 'en-US' | 'zh-CN'; // add other supported localespackages/kit/src/views/Prime/hooks/useFetchPrimeUserInfo.ts (1)
9-16: 🧹 Nitpick (assertive)
Add error handling to fetchPrimeUserInfo.
The function should catch and handle potential API errors.
const fetchPrimeUserInfo = useCallback(async () => { if (isReady && user?.id) { - const userInfo = - await backgroundApiProxy.servicePrime.apiFetchPrimeUserInfo(); - return userInfo; + try { + const userInfo = + await backgroundApiProxy.servicePrime.apiFetchPrimeUserInfo(); + return userInfo; + } catch (error) { + console.error('Failed to fetch Prime user info:', error); + throw error; + } } return undefined; }, [isReady, user?.id]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const fetchPrimeUserInfo = useCallback(async () => { if (isReady && user?.id) { try { const userInfo = await backgroundApiProxy.servicePrime.apiFetchPrimeUserInfo(); return userInfo; } catch (error) { console.error('Failed to fetch Prime user info:', error); throw error; } } return undefined; }, [isReady, user?.id]);packages/shared/types/prime/primeTypes.ts (1)
1-11: 🧹 Nitpick (assertive)
Add JSDoc documentation to types.
Add descriptions to help other developers understand the purpose and usage of these types.
+/** + * Information about a user's Prime subscription status + */ export type IPrimeSubscriptionInfo = { isActive: boolean; expiresAt: number; }; + +/** + * Comprehensive user information including login status and subscription details + */ export type IPrimeUserInfo = { isLoggedIn: boolean; email: string | undefined; privyUserId: string | undefined; subscriptionManageUrl: string | undefined; primeSubscription: IPrimeSubscriptionInfo | undefined; };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements./** * Information about a user's Prime subscription status */ export type IPrimeSubscriptionInfo = { isActive: boolean; expiresAt: number; }; /** * Comprehensive user information including login status and subscription details */ export type IPrimeUserInfo = { isLoggedIn: boolean; email: string | undefined; privyUserId: string | undefined; subscriptionManageUrl: string | undefined; primeSubscription: IPrimeSubscriptionInfo | undefined; };packages/kit/src/views/Prime/purchasesSdk/PurchasesSdkWeb.ts (2)
13-15: 🧹 Nitpick (assertive)
Improve error message clarity
Make the error message more specific about which API key is missing.
- throw new Error('No REVENUECAT api key found'); + throw new Error( + `RevenueCat API key not found for ${ + devSettings?.settings?.usePrimeSandboxPayment ? 'sandbox' : 'production' + } environment` + );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if (!apiKey) { - throw new Error('No REVENUECAT api key found'); + throw new Error( + `RevenueCat API key not found for ${ + devSettings?.settings?.usePrimeSandboxPayment ? 'sandbox' : 'production' + } environment` + ); }
9-12: 💡 Verification agent
🧩 Analysis chain
Add fallback for missing environment variables
The code assumes environment variables exist. Add explicit checks for both production and sandbox API keys.
🏁 Script executed:
#!/bin/bash # Check if environment variables are defined in .env files rg "REVENUECAT_API_KEY_WEB" -l rg "REVENUECAT_API_KEY_WEB_SANDBOX" -lLength of output: 841
Action Required: Implement Explicit Fallback for Missing Environment Variables
The current implementation in packages/kit/src/views/Prime/purchasesSdk/PurchasesSdkWeb.ts directly retrieves API keys without checking if they are defined. Even though the environment variables exist in various parts of the repo, the code does not safeguard against a missing key.
Consider updating the code as follows:
- Check if process.env.REVENUECAT_API_KEY_WEB is defined before proceeding. If it’s missing, provide a clear error message or a safe fallback.
- Likewise, when the sandbox mode is enabled, verify process.env.REVENUECAT_API_KEY_WEB_SANDBOX is present.
- Example adjustment:
const prodApiKey = process.env.REVENUECAT_API_KEY_WEB; if (!prodApiKey) { throw new Error('Missing RevenueCat API key for production.'); } let apiKey = prodApiKey; if (devSettings?.settings?.usePrimeSandboxPayment) { const sandboxApiKey = process.env.REVENUECAT_API_KEY_WEB_SANDBOX; if (!sandboxApiKey) { throw new Error('Missing RevenueCat API key for sandbox mode.'); } apiKey = sandboxApiKey; }These explicit checks will enhance security and help prevent runtime issues if an expected environment variable is missing.
packages/kit-bg/src/dbs/simple/entity/SimpleDbEntityPrime.ts (2)
5-7: 🛠️ Refactor suggestion
Consider adding token expiry
Add expiration tracking for the auth token to enhance security.
export interface ISimpleDBPrime { authToken: string; + tokenExpiry: number; }
19-25: 🛠️ Refactor suggestion
Add token validation
Add input validation for the auth token before saving.
@backgroundMethod() async saveAuthToken(authToken: string) { + if (!authToken || typeof authToken !== 'string') { + throw new Error('Invalid auth token'); + } await this.setRawData((v) => ({ ...v, authToken, })); }development/env.js (1)
17-18: 🧹 Nitpick (assertive)
Consider using a debug logger
Replace console.log with a proper debug logger for better control over development logging.
- // console.log('process.env', process.env); + // debug('Environment variables: %O', process.env);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.// debug('Environment variables: %O', process.env);packages/shared/src/googlePlayService/googlePlayService.android.ts (2)
11-15: 🧹 Nitpick (assertive)
Improve logging format
Use structured logging for better parsing and filtering.
- console.log( - 'googlePlayService: isAvailable result >>>>> ', - hasPlayServices, - ); + console.log({ + service: 'googlePlayService', + method: 'isAvailable', + result: hasPlayServices + });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.console.log({ service: 'googlePlayService', method: 'isAvailable', result: hasPlayServices }); return hasPlayServices;
17-18: 🧹 Nitpick (assertive)
Add error type information
Log the error type for better debugging.
- console.error('googlePlayService: isAvailable error >>>>> ', e); + console.error({ + service: 'googlePlayService', + method: 'isAvailable', + error: e instanceof Error ? e.message : String(e), + errorType: e?.constructor?.name + });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.console.error({ service: 'googlePlayService', method: 'isAvailable', error: e instanceof Error ? e.message : String(e), errorType: e?.constructor?.name }); return false;packages/components/src/content/LottieView/type.tsx (1)
17-17: 🛠️ Refactor suggestion
Remove redundant type union
Line 17 is redundant since width and height are already optional in lines 15-16.
- } & ({ width?: StackProps['width'] } | { height?: StackProps['height'] });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.apps/ext/src/ui/renderApp.tsx (1)
8-8: 🧹 Nitpick (assertive)
Move import to group
Move this import to the existing import group below to maintain consistent ordering.
-import { SentryErrorBoundaryFallback } from '@onekeyhq/kit/src/components/ErrorBoundary'; import { initSentry, withSentryHOC, } from '@onekeyhq/shared/src/modules3rdParty/sentry'; +import { SentryErrorBoundaryFallback } from '@onekeyhq/kit/src/components/ErrorBoundary';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.import { initSentry, withSentryHOC, } from '@onekeyhq/shared/src/modules3rdParty/sentry'; import { SentryErrorBoundaryFallback } from '@onekeyhq/kit/src/components/ErrorBoundary';packages/kit/src/views/Prime/hooks/usePrivyUniversalV2/usePrivyUniversalV2Types.ts (1)
18-19: 🧹 Nitpick (assertive)
Replace 'any' with a specific error type.
Define a custom error type or use
Errortype for better type safety.- onError?: (error: any) => void; + onError?: (error: Error) => void;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.onError?: (error: Error) => void; }) => {packages/components/src/primitives/Icon/react/colored/OnekeyPrimeDark.tsx (1)
9-9: 🧹 Nitpick (assertive)
Improve accessibility and gradient ID.
Add aria-label and use more descriptive gradient ID.
- <Svg viewBox="0 0 24 24" fill="none" accessibilityRole="image" {...props}> + <Svg viewBox="0 0 24 24" fill="none" accessibilityRole="image" aria-label="OneKey Prime Dark" {...props}>- id="a" + id="prime_dark_gradient"Also applies to: 19-19
packages/kit/src/views/Prime/hooks/usePrimeAuthV2.tsx (1)
14-20: 🧹 Nitpick (assertive)
Add error logging for failed API calls.
The error handling is good, but we should log API failures.
const logoutWithApi: () => Promise<void> = useCallback(async () => { try { await backgroundApiProxy.servicePrime.apiLogout(); } catch (error) { + console.error('Prime logout API failed:', error); } finally { await logout(); } }, [logout]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const logoutWithApi: () => Promise<void> = useCallback(async () => { try { await backgroundApiProxy.servicePrime.apiLogout(); } catch (error) { console.error('Prime logout API failed:', error); } finally { await logout(); } }, [logout]);packages/kit/src/views/Prime/pages/PrimeDashboard/PrimeLottieAnimation.tsx (1)
22-35: 🧹 Nitpick (assertive)
Consider using transform3d for better performance.
Using CSS transform3d triggers hardware acceleration.
<Stack style={{ position: 'absolute', top: '50%', - transform: 'translateY(-50%)', + transform: 'translate3d(0, -50%, 0)', left: 0, right: 0, paddingBottom: '100%', }} >📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.return ( <Stack style={{ position: 'absolute', top: '50%', transform: 'translate3d(0, -50%, 0)', left: 0, right: 0, paddingBottom: '100%', }} > <PrimeLottie /> </Stack> );packages/kit/src/views/Prime/components/PrivyProvider.native.tsx (2)
18-23: 🧹 Nitpick (assertive)
Improve error messages.
Add more context to error messages to help with debugging.
if (!appId) { - throw new Error('PRIVY_APP_ID is not set'); + throw new Error('Configuration error: PRIVY_APP_ID environment variable is required'); } if (!clientId) { - throw new Error('PRIVY_MOBILE_CLIENT_ID is not set'); + throw new Error('Configuration error: PRIVY_MOBILE_CLIENT_ID environment variable is required'); }
14-17: 🛠️ Refactor suggestion
Move hardcoded values to constants.
Hardcoded IDs should be in a constants file for better maintainability.
+const DEFAULT_APP_ID = 'cm6c9xup40017zyrnnp8zh0bt'; +const DEFAULT_CLIENT_ID = 'client-WY5gESiXQgTXogYv2M8iCM3LaaDDaKAdigE9Bg7a9pr1W'; + export function PrivyProvider({ children }: { children: React.ReactNode }) { - const appId = process.env.PRIVY_APP_ID || 'cm6c9xup40017zyrnnp8zh0bt'; + const appId = process.env.PRIVY_APP_ID || DEFAULT_APP_ID; const clientId = - process.env.PRIVY_MOBILE_CLIENT_ID || - 'client-WY5gESiXQgTXogYv2M8iCM3LaaDDaKAdigE9Bg7a9pr1W'; + process.env.PRIVY_MOBILE_CLIENT_ID || DEFAULT_CLIENT_ID;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const DEFAULT_APP_ID = 'cm6c9xup40017zyrnnp8zh0bt'; const DEFAULT_CLIENT_ID = 'client-WY5gESiXQgTXogYv2M8iCM3LaaDDaKAdigE9Bg7a9pr1W'; export function PrivyProvider({ children }: { children: React.ReactNode }) { const appId = process.env.PRIVY_APP_ID || DEFAULT_APP_ID; const clientId = process.env.PRIVY_MOBILE_CLIENT_ID || DEFAULT_CLIENT_ID; // ...rest of the implementation }development/scripts/web-embed.js (2)
30-33:
⚠️ Potential issueRemove API key logging
Logging API keys is a security risk. They could appear in build logs or console output.
- console.log( - 'process.env.REVENUECAT_API_KEY_WEB_SANDBOX', - process.env.REVENUECAT_API_KEY_WEB_SANDBOX, - );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
7-7: 🧹 Nitpick (assertive)
Add API key validation
Check if required API keys exist before building.
require('../env'); +if (!process.env.REVENUECAT_API_KEY_WEB_SANDBOX) { + console.error('Missing REVENUECAT_API_KEY_WEB_SANDBOX'); + exit(1); +}Committable suggestion skipped: line range outside the PR's diff.
packages/kit/src/views/Prime/hooks/usePrivyUniversal/usePrivyUniversalTypes.ts (1)
5-29: 🧹 Nitpick (assertive)
Add JSDoc comments
Add documentation to explain the type's purpose and properties.
+/** + * Universal interface for Privy authentication across native and web platforms. + * @property native - Native platform specific authentication methods and state + * @property web - Web platform specific authentication methods and state + * @property userEmail - User's email address + * @property privyUserId - Unique Privy user identifier + * @property authenticated - Authentication status + * @property isReady - Initialization status + * @property logout - Logout method + * @property getAccessToken - Access token retrieval method + */ export type IUsePrivyUniversal = {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements./** * Universal interface for Privy authentication across native and web platforms. * @property native - Native platform specific authentication methods and state * @property web - Web platform specific authentication methods and state * @property userEmail - User's email address * @property privyUserId - Unique Privy user identifier * @property authenticated - Authentication status * @property isReady - Initialization status * @property logout - Logout method * @property getAccessToken - Access token retrieval method */ export type IUsePrivyUniversal = { native: | { error: Error | null | undefined; user: PrivyUser | null | undefined; login: ((config: LoginUIConfig) => void) | undefined; } | undefined; web: | { user: User | null | undefined; login: | ((options?: LoginModalOptions | React.MouseEvent<any, any>) => void) | undefined; updateEmail: (() => void) | undefined; // web only updatePhone: (() => void) | undefined; // web only } | undefined; userEmail: string | undefined; privyUserId: string | undefined; authenticated: boolean; isReady: boolean; logout: () => Promise<void>; getAccessToken: () => Promise<string | null>; };packages/shared/src/modules3rdParty/sentry/index.desktop.ts (1)
32-34: 🧹 Nitpick (assertive)
Improve error logging
Replace console.error with proper error tracking in production.
onError: (error, info) => { - console.error('error', error, info); + if (process.env.NODE_ENV === 'development') { + console.error('error', error, info); + } + Sentry.captureException(error, { extra: info }); },packages/shared/src/modules3rdParty/sentry/index.ext.ts (1)
31-36: 🧹 Nitpick (assertive)
Extract shared error handling logic
This code is duplicated in index.desktop.ts. Create a shared utility.
Create a new file
errorHandling.ts:export const createErrorBoundaryOptions = (errorBoundaryFallback?: FallbackRender) => ({ onError: (error: Error, info: any) => { if (process.env.NODE_ENV === 'development') { console.error('error', error, info); } Sentry.captureException(error, { extra: info }); }, fallback: errorBoundaryFallback, });Then use it in both files:
- Sentry.withErrorBoundary(Sentry.withProfiler(Component), { - onError: (error, info) => { - console.error('error', error, info); - }, - fallback: errorBoundaryFallback, - }); + Sentry.withErrorBoundary( + Sentry.withProfiler(Component), + createErrorBoundaryOptions(errorBoundaryFallback) + );packages/kit/src/views/Prime/components/PrimeDeviceLogoutAlertDialog.tsx (2)
25-28: 🧹 Nitpick (assertive)
Fix grammar in alert message
The text has a grammar error. Change "This device been deactivated" to "This device has been deactivated".
- This device been deactivated from another device. If this wasn't you, + This device has been deactivated from another device. If this wasn't you,
34-36: 🧹 Nitpick (assertive)
Remove empty callback
The cancel button has an empty async callback. Since no action is needed, you can remove the callback entirely.
- onCancel={async () => { - // - }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.packages/kit/src/views/Prime/hooks/usePrivyUniversalV2/usePrivyUniversalV2.ts (1)
13-15: 🧹 Nitpick (assertive)
Add type for error parameter
The error callback lacks type information. Consider adding a specific error type to improve type safety.
- onError: (error) => { + onError: (error: Error) => {packages/kit/src/views/Prime/hooks/usePrivyUniversalV2/usePrivyUniversalV2.native.ts (2)
5-39: 🧹 Nitpick (assertive)
Consider sharing common logic
This hook shares significant code with its web counterpart. Consider extracting common logic into a shared utility function.
33-36: 🧹 Nitpick (assertive)
Add null check for user ID
The user ID access could be unsafe. Add a null check or use nullish coalescing.
- id: user?.id || '', + id: user?.id ?? '',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.user: authenticated ? { id: user?.id ?? '', }packages/kit/src/provider/Container/DialogLoadingContainer/DialogLoadingContainer.tsx (2)
17-22: 🧹 Nitpick (assertive)
Add error handling for dialog operations
The
showFnshould handle potential failures in dialog operations.const showFn = async ( payload: IAppEventBusPayload[EAppEventBusNames.ShowDialogLoading], ) => { + try { await hideFn(); dialogRef.current = Dialog.loading(payload); + } catch (error) { + console.error('Failed to show loading dialog:', error); + } };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const showFn = async ( payload: IAppEventBusPayload[EAppEventBusNames.ShowDialogLoading], ) => { try { await hideFn(); dialogRef.current = Dialog.loading(payload); } catch (error) { console.error('Failed to show loading dialog:', error); } };
14-16: 🧹 Nitpick (assertive)
Add cleanup for in-flight async operations
The async
hideFncould leak if the component unmounts during execution.const hideFn = async () => { + if (!dialogRef.current) return; await dialogRef.current?.close(); + dialogRef.current = null; };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const hideFn = async () => { if (!dialogRef.current) return; await dialogRef.current?.close(); dialogRef.current = null; };packages/shared/src/modules3rdParty/sentry/index.ts (2)
27-28: 🧹 Nitpick (assertive)
Define specific type for breadcrumb args
Using
anytype reduces type safety. Consider using Sentry's breadcrumb types.-export const addBreadcrumb = (args: any) => {}; +import type { Breadcrumb } from '@sentry/types'; +export const addBreadcrumb = (args: Breadcrumb) => {};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.// eslint-disable-next-line @typescript-eslint/no-unused-vars import type { Breadcrumb } from '@sentry/types'; export const addBreadcrumb = (args: Breadcrumb) => {};
35-37: 🧹 Nitpick (assertive)
Enhance error logging
Add more context to error logs for better debugging.
onError: (error, info) => { - console.error('error', error, info); + console.error('Sentry error boundary caught error:', { + error, + componentStack: info.componentStack, + }); },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.onError: (error, info) => { console.error('Sentry error boundary caught error:', { error, componentStack: info.componentStack, }); },development/webpack/webpack.analyzer.config.js (1)
8-9: 🧹 Nitpick (assertive)
Improve log readability
Make console logs more distinct and informative.
-console.log('ENABLE_ANALYZER_HTML_REPORT >>>>>> ', ENABLE_ANALYZER_HTML_REPORT); -console.log('ENABLE_ANALYZER >>>>>> ', ENABLE_ANALYZER); +console.log('[Webpack Analyzer] HTML Report enabled:', ENABLE_ANALYZER_HTML_REPORT); +console.log('[Webpack Analyzer] Analyzer enabled:', ENABLE_ANALYZER);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.console.log('[Webpack Analyzer] HTML Report enabled:', ENABLE_ANALYZER_HTML_REPORT); console.log('[Webpack Analyzer] Analyzer enabled:', ENABLE_ANALYZER);packages/kit/src/views/Prime/hooks/usePrivyUniversal/usePrivyUniversal.ts (2)
31-32: 🧹 Nitpick (assertive)
Add null checks for user properties
Add optional chaining to prevent potential runtime errors.
-userEmail: privy.user?.email?.address, -privyUserId: privy.user?.id, +userEmail: privy.user?.email?.address ?? null, +privyUserId: privy.user?.id ?? null,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.userEmail: privy.user?.email?.address ?? null, privyUserId: privy.user?.id ?? null,
16-19: 🧹 Nitpick (assertive)
Improve login completion logging
Replace console log with proper logging utility and add environment check.
onComplete(...args) { - console.log('privy login complete >>> ', args); + if (process.env.NODE_ENV !== 'production') { + console.log('[Privy] Login completed:', args); + } },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.onComplete(...args) { if (process.env.NODE_ENV !== 'production') { console.log('[Privy] Login completed:', args); } }, });packages/kit/src/views/Prime/hooks/usePrivyUniversal/usePrivyUniversal.native.ts (2)
12-12: 🧹 Nitpick (assertive)
Remove console.log statement
Remove debug logging before deploying to production.
- console.log('usePrivyUniversal: isReady >>>>> ', isReady);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.// (The console.log statement has been removed)
16-21: 🧹 Nitpick (assertive)
Use type guard instead of type assertion
Replace unsafe type assertion with a type guard for better type safety.
- const emailUser = user?.linked_accounts?.find( - (item) => item.type === 'email', - ); - if (emailUser) { - const address: string = (emailUser as { address: string }).address; - return address; + const emailUser = user?.linked_accounts?.find( + (item): item is { type: 'email'; address: string } => + item.type === 'email' && 'address' in item + ); + if (emailUser) { + return emailUser.address;packages/kit/src/views/Prime/components/PrivyProvider.tsx (1)
11-14:
⚠️ Potential issueRemove hardcoded credentials
Move fallback credentials to environment variables. Hardcoding sensitive values in source code is a security risk.
- const appId = process.env.PRIVY_APP_ID || 'cm6c9xup40017zyrnnp8zh0bt'; - const clientId = - process.env.PRIVY_MOBILE_CLIENT_ID || - 'client-WY5gESiXQgTXogYv2M8iCM3LaaDDaKAdigGMM4dCxSzy6'; + const appId = process.env.PRIVY_APP_ID; + const clientId = process.env.PRIVY_MOBILE_CLIENT_ID;packages/kit/src/views/Prime/hooks/PrimeSignupTest.tsx (2)
8-8: 🧹 Nitpick (assertive)
Remove hardcoded test email
Remove the hardcoded email address from the initial state.
- const [email, setEmail] = useState('[email protected]'); + const [email, setEmail] = useState('');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const [email, setEmail] = useState('');
21-25: 🛠️ Refactor suggestion
Improve error handling
Console logging isn't sufficient for error handling. Display error message to user.
onError(error) { - console.log('error', error); - // show a toast, update form errors, etc... + Toast.error({ + title: 'Login Error', + message: error.message, + }); },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.onError(error) { Toast.error({ title: 'Login Error', message: error.message, }); }, });development/envExposedToClient.js (1)
31-31: 🧹 Nitpick (assertive)
Add links to workflow files
Make the comment more helpful by adding direct links to the workflow files.
- // *** also update Inject Environment Variables at release-ios.yml, release-android *** + // *** also update Inject Environment Variables at: + // *** - .github/workflows/release-ios.yml + // *** - .github/workflows/release-android.yml ***📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.// *** also update Inject Environment Variables at: // *** - .github/workflows/release-ios.yml // *** - .github/workflows/release-android.yml ***packages/kit/src/views/Developer/pages/Gallery/Components/stories/WebView.tsx (2)
22-24:
⚠️ Potential issueRemove hardcoded test values
Replace hardcoded test values with proper test data or environment variables.
- primeUserId: 'hello-world-1234567890', - primeUserEmail: '[email protected]', + primeUserId: process.env.PRIME_TEST_USER_ID, + primeUserEmail: process.env.PRIME_TEST_USER_EMAIL,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.primeUserId: process.env.PRIME_TEST_USER_ID, primeUserEmail: process.env.PRIME_TEST_USER_EMAIL, },
20-20: 🛠️ Refactor suggestion
Fix type casting
Avoid using
anytype. Define proper type forhashRoutePath.- hashRoutePath: hashRoutePath as any, + hashRoutePath: hashRoutePath as EWebEmbedRoutePath,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.hashRoutePath: hashRoutePath as EWebEmbedRoutePath,packages/shared/src/modules3rdParty/sentry/index.native.ts (1)
44-53: 🧹 Nitpick (assertive)
Add JSDoc comments
The implementation looks good. Add documentation for the new parameter.
+/** + * HOC that wraps a component with Sentry error boundary + * @param Component - The component to wrap + * @param errorBoundaryFallback - Optional custom fallback UI when an error occurs + * @returns Wrapped component with error boundary + */ export const withSentryHOC = ( Component: ComponentType<any>, errorBoundaryFallback?: FallbackRender, ): ComponentType<any> =>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements./** * HOC that wraps a component with Sentry error boundary * @param Component - The component to wrap * @param errorBoundaryFallback - Optional custom fallback UI when an error occurs * @returns Wrapped component with error boundary */ export const withSentryHOC = ( Component: ComponentType<any>, errorBoundaryFallback?: FallbackRender, ): ComponentType<any> => withErrorBoundary(withProfiler(wrap(Component)), { onError: (error, info) => { console.error('error', error, info); }, fallback: errorBoundaryFallback, });development/webpack/utils.js (1)
6-6: 🛠️ Refactor suggestion
Remove debug console.log
Debug logs should not be committed to production code. They clutter the console.
- console.log('createResolveExtensions>>>>>>', platform, configName);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.packages/kit/src/views/Prime/pages/PrimeDashboard/PrimeUserInfo.tsx (1)
33-36: 🧹 Nitpick (assertive)
Remove commented console.log statements
Clean up unused debug code.
- onPress={() => { - // console.log(privy?.web?.user); - // console.log(privy?.native?.user); - }} + onPress={() => {}}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.onPress={() => {}}packages/kit/src/views/Prime/components/PrimeHeaderIconButton/PrimeHeaderIconButton.tsx (1)
27-31: 🧹 Nitpick (assertive)
Improve error message clarity
The error message "Prime not ready" is too technical. Make it more user-friendly.
- title: 'Prime not ready.', + title: 'Please wait while Prime initializes...',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.Toast.message({ - title: 'Prime not ready.', + title: 'Please wait while Prime initializes...', }); return; }packages/kit-bg/src/services/ServiceNotification/NotificationProvider/NotificationProvider.web.ts (1)
18-18: 💡 Verification agent
🧩 Analysis chain
Verify WebSocket provider initialization
The WebSocket provider is now initialized but the implementation is missing. Ensure proper error handling and connection management.
🏁 Script executed:
#!/bin/bash # Check if WebSocket implementation exists rg -A 5 "initWebSocketProvider|WebSocket"Length of output: 30271
WebSocket Provider Initialization Needs Further Implementation
- The call to initWebSocketProvider() in NotificationProvider.web.ts properly initiates the WebSocket provider.
- The base class (NotificationProviderBase) and PushProviderWebSocket show that error handling and connection events (error, reconnect, disconnect, etc.) are managed.
- However, key methods in NotificationProvider.web.ts (e.g. showAndFocusApp) remain unimplemented, which could affect proper error handling or UI response in the web environment.
packages/kit-bg/src/states/jotai/atoms/prime.ts (1)
34-38: 🧹 Nitpick (assertive)
Add email validation type
The email field should use a more specific type for validation.
export type IPrimeLoginDialogAtomPasswordData = { isRegister?: boolean; - email: string; + email: `${string}@${string}.${string}`; promiseId: number; };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export type IPrimeLoginDialogAtomPasswordData = { isRegister?: boolean; email: `${string}@${string}.${string}`; promiseId: number; };packages/components/src/forms/OTPInput/index.tsx (1)
42-72: 🛠️ Refactor suggestion
Add accessibility props
The OTP input needs accessibility support for screen readers.
<OtpInput ref={ref} + accessibilityLabel="Enter verification code" + accessibilityHint="Enter the code sent to your email" theme={{📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.return ( <OtpInput ref={ref} accessibilityLabel="Enter verification code" accessibilityHint="Enter the code sent to your email" theme={{ pinCodeTextStyle: { fontSize: 20, fontWeight: 'bold', color: theme.text.val, }, pinCodeContainerStyle: { width: 50, height: 50, borderWidth: 1, borderColor: innerStatus === 'error' ? theme.red8.val : theme.neutral7.val, }, filledPinCodeContainerStyle: { borderWidth: 2, backgroundColor: theme.gray2.val, }, focusedPinCodeContainerStyle: { borderWidth: 2, borderColor: innerStatus === 'error' ? theme.red8.val : theme.borderActive.val, }, }} focusColor={theme.text.val} numberOfDigits={numberOfDigits} {...rest} /> );packages/kit/src/components/ErrorBoundary/index.tsx (2)
50-69: 🛠️ Refactor suggestion
Utilize all provided props for better error reporting.
The function receives componentStack, eventId, and resetError props but doesn't use them. These could provide valuable debug information and recovery options.
function SentryErrorBoundaryFallback({ error, componentStack, eventId, resetError, }: { error: unknown; componentStack: string; eventId: string; resetError(): void; }) { return ( - <SafeAreaView> - <Text> - {(error as Error | undefined)?.message || - 'unknown error by error boundary'} - </Text> + <SafeAreaView style={{ padding: 16 }}> + <Text style={{ fontSize: 16, color: 'red' }}> + {(error as Error | undefined)?.message || 'Unknown Error'} + </Text> + <Text style={{ marginTop: 8, fontSize: 14 }}>Event ID: {eventId}</Text> + <Text style={{ marginTop: 8, fontSize: 14 }}>{componentStack}</Text> + <Button onPress={resetError}>Try Again</Button> </SafeAreaView> ); }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.function SentryErrorBoundaryFallback({ error, componentStack, eventId, resetError, }: { error: unknown; componentStack: string; eventId: string; resetError(): void; }) { return ( <SafeAreaView style={{ padding: 16 }}> <Text style={{ fontSize: 16, color: 'red' }}> {(error as Error | undefined)?.message || 'Unknown Error'} </Text> <Text style={{ marginTop: 8, fontSize: 14 }}>Event ID: {eventId}</Text> <Text style={{ marginTop: 8, fontSize: 14 }}>{componentStack}</Text> <Button onPress={resetError}>Try Again</Button> </SafeAreaView> ); }
37-48: 🧹 Nitpick (assertive)
Enhance error message presentation.
The error UI is basic. Consider adding:
- Error message styling
- Error details expansion/collapse
- Retry button
class ErrorBoundarySimple extends ErrorBoundaryBase { override render() { if (this.state.error) { return ( - <SafeAreaView> - <Text>{this.state.error.message}</Text> + <SafeAreaView style={{ padding: 16 }}> + <Text style={{ fontSize: 16, color: 'red' }}>{this.state.error.message}</Text> + <Text style={{ marginTop: 8, fontSize: 14 }}> + {this.state.error.stack} + </Text> + <Button onPress={() => this.setState({ error: null })}> + Retry + </Button> </SafeAreaView> ); } return this.props.children; } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.class ErrorBoundarySimple extends ErrorBoundaryBase { override render() { if (this.state.error) { return ( <SafeAreaView style={{ padding: 16 }}> <Text style={{ fontSize: 16, color: 'red' }}>{this.state.error.message}</Text> <Text style={{ marginTop: 8, fontSize: 14 }}> {this.state.error.stack} </Text> <Button onPress={() => this.setState({ error: null })}> Retry </Button> </SafeAreaView> ); } return this.props.children; } }packages/kit/src/views/Prime/hooks/usePrivyAuthSyncToAtom.ts (2)
75-83:
⚠️ Potential issueAdd error handling to logout operation.
The logout operation could fail silently.
useEffect(() => { const fn = async () => { + try { await logout(); + } catch (error) { + console.error('Failed to logout:', error); + } }; appEventBus.on(EAppEventBusNames.PrimeLoginInvalidToken, fn); return () => { appEventBus.off(EAppEventBusNames.PrimeLoginInvalidToken, fn); }; }, [logout]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.useEffect(() => { const fn = async () => { try { await logout(); } catch (error) { console.error('Failed to logout:', error); } }; appEventBus.on(EAppEventBusNames.PrimeLoginInvalidToken, fn); return () => { appEventBus.off(EAppEventBusNames.PrimeLoginInvalidToken, fn); }; }, [logout]);
32-74:
⚠️ Potential issueAdd error handling to async operations.
The async function lacks try-catch blocks. Errors could silently fail.
useEffect(() => { void (async () => { + try { if (!isReady) { return; } let accessToken: string | null = ''; if (authenticated) { accessToken = await getAccessToken(); } await backgroundApiProxy.simpleDb.prime.saveAuthToken(accessToken || ''); if (accessToken) { setPrimePersistAtom((v) => ({ ...v, isLoggedIn: true, email: userEmail, privyUserId, })); } else { setPrimePersistAtom((v) => ({ ...v, isLoggedIn: false, email: undefined, privyUserId: undefined, })); } setPrimeInitAtom((v) => ({ ...v, isReady: true, })); + } catch (error) { + console.error('Failed to sync Privy auth state:', error); + setPrimeInitAtom((v) => ({ + ...v, + isReady: true, + error: error instanceof Error ? error.message : 'Unknown error', + })); + } })(); }, [ setPrimePersistAtom, setPrimeInitAtom, authenticated, getAccessToken, isReady, userEmail, privyUserId, ]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.useEffect(() => { void (async () => { try { if (!isReady) { return; } let accessToken: string | null = ''; if (authenticated) { accessToken = await getAccessToken(); } await backgroundApiProxy.simpleDb.prime.saveAuthToken(accessToken || ''); if (accessToken) { setPrimePersistAtom((v) => ({ ...v, isLoggedIn: true, email: userEmail, privyUserId, })); } else { setPrimePersistAtom((v) => ({ ...v, isLoggedIn: false, email: undefined, privyUserId: undefined, })); } setPrimeInitAtom((v) => ({ ...v, isReady: true, })); } catch (error) { console.error('Failed to sync Privy auth state:', error); setPrimeInitAtom((v) => ({ ...v, isReady: true, error: error instanceof Error ? error.message : 'Unknown error', })); } })(); }, [ setPrimePersistAtom, setPrimeInitAtom, authenticated, getAccessToken, isReady, userEmail, privyUserId, ]);apps/web-embed/pages/PageWebEmbedApi.ts (2)
53-65:
⚠️ Potential issueReplace global flag with useRef.
Using a global flag for component state is an anti-pattern.
-let isInitExecuted = false; const PageWebEmbedApi = memo(() => { + const isInitExecuted = useRef(false); useEffect(() => { - if (isInitExecuted) { + if (isInitExecuted.current) { return; } - isInitExecuted = true; + isInitExecuted.current = true; init(); printMessageToBody('web-embed init...'); printMessageToBody(`${globalThis.location.href}`); }, []); return null; });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const PageWebEmbedApi = memo(() => { const isInitExecuted = useRef(false); useEffect(() => { if (isInitExecuted.current) { return; } isInitExecuted.current = true; init(); printMessageToBody('web-embed init...'); printMessageToBody(`${globalThis.location.href}`); }, []); return null; });
9-16:
⚠️ Potential issueMove refresh button creation into component.
Creating DOM elements in the global scope is an anti-pattern. Move this into the component.
-// create button which can refresh, append to body -const refreshButton = document.createElement('button'); -refreshButton.textContent = 'Refresh'; -refreshButton.addEventListener('click', () => { - globalThis.location.reload(); -}); -document.body.appendChild(refreshButton); +const PageWebEmbedApi = memo(() => { + useEffect(() => { + const refreshButton = document.createElement('button'); + refreshButton.textContent = 'Refresh'; + refreshButton.addEventListener('click', () => { + globalThis.location.reload(); + }); + document.body.appendChild(refreshButton); + return () => { + refreshButton.remove(); + }; + }, []);Committable suggestion skipped: line range outside the PR's diff.
development/lint/eslint-rule-force-async-bg-api.js (3)
5-6: 🧹 Nitpick (assertive)
Translate comments to English.
Keep comments in English for better maintainability.
- type: 'problem', // 规则类型：problem, suggestion, layout + type: 'problem', // Rule type: problem, suggestion, layout📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.type: 'problem', // Rule type: problem, suggestion, layout docs: {
23-29: 🧹 Nitpick (assertive)
Use optional chaining for better readability.
The code can be simplified using optional chaining.
- const hasBackgroundMethod = - node.decorators && - node.decorators.some( - (decorator) => - decorator.expression.type === 'CallExpression' && - decorator.expression.callee.name === 'backgroundMethod', - ); + const hasBackgroundMethod = node.decorators?.some( + (decorator) => + decorator.expression.type === 'CallExpression' && + decorator.expression.callee.name === 'backgroundMethod', + );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const hasBackgroundMethod = node.decorators?.some( (decorator) => decorator.expression.type === 'CallExpression' && decorator.expression.callee.name === 'backgroundMethod', );
43-50: 🧹 Nitpick (assertive)
Use optional chaining consistently.
Apply the same optional chaining pattern here.
- const hasBackgroundMethod = - node.parent.type === 'MethodDefinition' && - node.parent.decorators && - node.parent.decorators.some( - (decorator) => - decorator.expression.type === 'CallExpression' && - decorator.expression.callee.name === 'backgroundMethod', - ); + const hasBackgroundMethod = + node.parent.type === 'MethodDefinition' && + node.parent.decorators?.some( + (decorator) => + decorator.expression.type === 'CallExpression' && + decorator.expression.callee.name === 'backgroundMethod', + );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const hasBackgroundMethod = node.parent.type === 'MethodDefinition' && node.parent.decorators?.some( (decorator) => decorator.expression.type === 'CallExpression' && decorator.expression.callee.name === 'backgroundMethod', );packages/kit/src/views/Prime/pages/PrimeDashboard/PrimeBenefitsList.tsx (3)
67-67:
⚠️ Potential issueReplace placeholder text in Analytics subtitle
The subtitle "sint occaecat cupidatat non proident" appears to be Lorem ipsum placeholder text.
Replace with actual descriptive text about the analytics feature.
38-42: 🧹 Nitpick (assertive)
Enhance Toast messages with meaningful feedback
Toast messages only repeat the title without providing useful feedback to users.
Add more descriptive messages. For example:
- Toast.success({ - title: 'Sync', - }); + Toast.success({ + title: 'Sync', + message: 'Opening sync settings...', + });Also applies to: 48-52, 58-62, 68-72, 78-82
5-29: 🧹 Nitpick (assertive)
Consider extracting icon styles to a reusable component
The YStack styling for icons is repeated across multiple instances.
Create a reusable IconContainer component:
function IconContainer({ children }: { children: React.ReactNode }) { return ( <YStack borderRadius="$3" borderCurve="continuous" bg="$brand4" p="$2"> {children} </YStack> ); }packages/kit/src/views/Setting/utils/devMode.ts (1)
74-76:
⚠️ Potential issueLog errors in catch block
Empty catch block silently swallows errors.
Add error logging:
- } catch (error) { - /* empty */ + } catch (error) { + console.error('Failed to enable dev mode:', error);packages/shared/src/utils/openUrlUtils.ts (1)
32-58:
⚠️ Potential issueAdd URL validation
The function accepts URLs without validation.
Add URL validation before opening:
function openUrlByWebviewPro({ url, title, isWebEmbed, hashRoutePath, hashRouteQueryParams, }: { url: string; title: string; isWebEmbed?: boolean; hashRoutePath?: EWebEmbedRoutePath; hashRouteQueryParams?: Record<string, string>; }) { + try { + new URL(url); + } catch (e) { + throw new Error('Invalid URL provided'); + } appGlobals.$navigationRef.current?.navigate(ERootRoutes.Modal, {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.function openUrlByWebviewPro({ url, title, isWebEmbed, hashRoutePath, hashRouteQueryParams, }: { url: string; title: string; isWebEmbed?: boolean; hashRoutePath?: EWebEmbedRoutePath; hashRouteQueryParams?: Record<string, string>; }) { try { new URL(url); } catch (e) { throw new Error('Invalid URL provided'); } appGlobals.$navigationRef.current?.navigate(ERootRoutes.Modal, { screen: EModalRoutes.WebViewModal, params: { screen: EModalWebViewRoutes.WebView, params: { url, title, isWebEmbed, hashRoutePath, hashRouteQueryParams, }, }, }); }packages/kit-bg/src/services/ServiceBase.ts (2)
73-83: 🛠️ Refactor suggestion
Improve error handling in withDialogLoading
The function swallows errors silently.
Add error handling:
async withDialogLoading<T>( payload: IAppEventBusPayload[EAppEventBusNames.ShowDialogLoading], fn: () => Promise<T>, ) { try { this.showDialogLoading(payload); return await fn(); } catch (error) { + console.error('Operation failed:', error); + throw error; } finally { this.hideDialogLoading(); } }
63-67: 🧹 Nitpick (assertive)
Add return type annotation
Method lacks return type annotation.
Add void return type:
- showDialogLoading( + showDialogLoading(): void { payload: IAppEventBusPayload[EAppEventBusNames.ShowDialogLoading], ) {Committable suggestion skipped: line range outside the PR's diff.
packages/kit/src/views/Prime/hooks/usePrimeAuth.ts (1)
38-38: 🧹 Nitpick (assertive)
Remove debug console.log statement.
Production code should not contain debug logging.
- console.log('prime email >>> ', email);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.packages/kit/src/views/Prime/components/PrimeLoginEmailDialog/PrimeLoginEmailDialog.tsx (2)
51-59: 🧹 Nitpick (assertive)
Enhance email validation error messages
The error messages could be more user-friendly and internationalized.
- return 'email is required'; + return intl.formatMessage({ id: 'msg__email_required' }); - return 'invalid email'; + return intl.formatMessage({ id: 'msg__invalid_email' });
75-76: 🧹 Nitpick (assertive)
Consider debouncing the trim operation
The trim operation on every text change could affect performance with rapid typing.
- onChangeText={(text) => text?.trim() ?? text} + onChangeText={useCallback( + (text) => text?.trim() ?? text, + [] + )}Committable suggestion skipped: line range outside the PR's diff.
packages/kit/src/provider/index.tsx (2)
55-59: 🛠️ Refactor suggestion
Add font loading error handling
The
useFontshook result should be checked to handle font loading failures gracefully.- useFonts({ + const [fontsLoaded, error] = useFonts({ Inter_400Regular, Inter_500Medium, Inter_600SemiBold, }); + + if (error) { + console.error('Error loading fonts:', error); + } + + if (!fontsLoaded) { + return null; // or a loading indicator + }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const [fontsLoaded, error] = useFonts({ Inter_400Regular, Inter_500Medium, Inter_600SemiBold, }); if (error) { console.error('Error loading fonts:', error); } if (!fontsLoaded) { return null; // or a loading indicator }
62-82: 🧹 Nitpick (assertive)
Consider memoizing the content
The content JSX structure could be memoized to prevent unnecessary re-renders.
+ const content = useMemo(() => ( <SafeAreaProvider> ... </SafeAreaProvider> - ); + ), []);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const content = useMemo(() => ( <SafeAreaProvider> <PrivyProvider> <GlobalJotaiReady> <GestureHandlerRootView style={flexStyle}> <ThemeProvider> <NetworkReachabilityTracker /> <SplashProvider> <Container /> </SplashProvider> <PasswordVerifyPromptMount /> <WebViewWebEmbedProvider /> <LastActivityTracker /> <SystemLocaleTracker /> <StateActiveContainer /> <InAppNotification /> <SyncHomeAccountToDappAccountProvider /> </ThemeProvider> </GestureHandlerRootView> </GlobalJotaiReady> </PrivyProvider> </SafeAreaProvider> ), []);packages/shared/src/utils/stringUtils.ts (1)
103-108: 🧹 Nitpick (assertive)
Consider making isValidEmail more flexible.
The function is well-implemented but could be more reusable.
Consider this change:
-function isValidEmail(email: string): boolean { +function isValidEmail(email: string | undefined | null): boolean { if (!email || !isString(email)) { return false; } return validator.isEmail(email); }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.function isValidEmail(email: string | undefined | null): boolean { if (!email || !isString(email)) { return false; } return validator.isEmail(email); }packages/kit/src/views/Prime/pages/PrimeDeviceLimit/PrimeDeviceLimit.tsx (2)
60-60: 🧹 Nitpick (assertive)
Remove debug console.log statement.
Production code should not contain console.log statements.
- console.log(devices);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
51-55: 🧹 Nitpick (assertive)
Add error handling for device logout.
The logout operation should handle potential API failures gracefully.
onConfirm: async () => { + try { await backgroundApiProxy.servicePrime.apiLogoutPrimeUserDevice({ instanceId, }); navigation.popStack(); + } catch (error) { + Dialog.show({ + title: 'Logout Failed', + description: 'Failed to logout device. Please try again.', + }); + } },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.onConfirm: async () => { try { await backgroundApiProxy.servicePrime.apiLogoutPrimeUserDevice({ instanceId, }); navigation.popStack(); } catch (error) { Dialog.show({ title: 'Logout Failed', description: 'Failed to logout device. Please try again.', }); } },packages/kit/src/views/Prime/components/PrimeLoginEmailCodeDialogV2/PrimeLoginEmailCodeDialogV2.tsx (3)
40-48: 🛠️ Refactor suggestion
Add error handling to
sendEmailVerificationCode.The function swallows errors silently. Add proper error handling to inform users when code sending fails.
try { console.log('sendCode', email); void sendCode({ email }); setCountdown(COUNTDOWN_TIME); + } catch (error) { + // Show error to user + console.error('Failed to send verification code:', error); } finally { setIsResending(false); }
36-38:
⚠️ Potential issueFix the incorrect order of operations in
sendEmailVerificationCode.The
isResendingcheck happens after settingisResendingto true, making it unreachable.const sendEmailVerificationCode = useCallback(async () => { + if (isResending) { + return; + } setIsResending(true); - - if (isResending) { - return; - }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const sendEmailVerificationCode = useCallback(async () => { + if (isResending) { + return; + } setIsResending(true); // ... rest of the code remains unchanged ... }, [isResending]);
103-111: 🧹 Nitpick (assertive)
Consider adding validation feedback.
The OTP input could benefit from immediate validation feedback as users type.
<OTPInput status={state.status === 'error' ? 'error' : 'normal'} numberOfDigits={6} value={verificationCode} onTextChange={(value) => { setState({ status: 'initial' }); setVerificationCode(value); + if (value.length === 6 && !/^\d{6}$/.test(value)) { + setState({ status: 'error' }); + } }} />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.<OTPInput status={state.status === 'error' ? 'error' : 'normal'} numberOfDigits={6} value={verificationCode} onTextChange={(value) => { setState({ status: 'initial' }); setVerificationCode(value); if (value.length === 6 && !/^\d{6}$/.test(value)) { setState({ status: 'error' }); } }} />packages/kit/src/views/Prime/components/PrimeLoginEmailCodeDialog/PrimeLoginEmailCodeDialog.tsx (2)
106-112:
⚠️ Potential issueRemove debug toast notification.
The toast showing verification code in plain text poses a security risk.
onComplete={(value) => { setCode(value); - Toast.success({ - title: 'code', - message: value, - }); }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.onComplete={(value) => { setCode(value); }}
127-130:
⚠️ Potential issueImprove error handling in dialog confirmation.
The error is thrown after
preventClose(), which could lead to unhandled rejections.} catch (error) { preventClose(); - throw error; + Toast.error({ + title: 'Error', + message: 'Failed to verify code. Please try again.', + }); }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.} catch (error) { preventClose(); Toast.error({ title: 'Error', message: 'Failed to verify code. Please try again.', }); }packages/shared/src/utils/promiseUtils.ts (1)
153-156: 🧹 Nitpick (assertive)
Consider using private field syntax.
Use modern JavaScript private field syntax (#) for better encapsulation.
- _resolveFn: ((value: T) => void) | undefined; + #resolveFn: ((value: T) => void) | undefined; ready = new Promise<T>((resolve) => { - this._resolveFn = resolve; + this.#resolveFn = resolve; });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.ready = new Promise<T>((resolve) => { - this._resolveFn = resolve; + this.#resolveFn = resolve; }); - _resolveFn: ((value: T) => void) | undefined; + #resolveFn: ((value: T) => void) | undefined;packages/kit/src/views/WebView/pages/WebViewModal/index.tsx (1)
106-114: 🧹 Nitpick (assertive)
Add error handling to the message handler.
The webembedCustomReceiveHandler should handle malformed payloads and unknown methods gracefully.
const webembedCustomReceiveHandler = useCallback( (payload: IJsBridgeMessagePayload) => { + try { const data = payload.data as IJsonRpcRequest; if (data.method === 'wallet_closeWebViewModal') { navigation.pop(); } + } catch (error) { + console.error('Failed to process webembed message:', error); + } }, [navigation], );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const webembedCustomReceiveHandler = useCallback( (payload: IJsBridgeMessagePayload) => { try { const data = payload.data as IJsonRpcRequest; if (data.method === 'wallet_closeWebViewModal') { navigation.pop(); } } catch (error) { console.error('Failed to process webembed message:', error); } }, [navigation], );packages/kit/src/views/Developer/pages/Gallery/Components/stories/Logger.tsx (1)
109-129:
⚠️ Potential issueFix potential precision loss in Int toString.
The regular integer version will lose precision at runtime. Use BigInt consistently for large numbers.
Apply this fix:
- // eslint-disable-next-line no-loss-of-precision, @typescript-eslint/no-loss-of-precision - (1_234_567_890_123_456_789_077_777_777_777_777_777_799_999_999_999_998_888_888).toString(), + 1_234_567_890_123_456_789_077_777_777_777_777_777_799_999_999_999_998_888_888n.toString(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.<Button onPress={() => { console.log( 'BigInt toString: ', 1_234_567_890_123_456_789_077_777_777_777_777_777_799_999_999_999_998_888_888n.toString(), ); }} > BigInt toString </Button> <Button onPress={() => { console.log( 'Int toString: ', 1_234_567_890_123_456_789_077_777_777_777_777_777_799_999_999_999_998_888_888n.toString(), ); }} > Int toString </Button>🧰 Tools
🪛 Biome (1.9.4)
[error] 124-124: This number literal will lose precision at runtime.
The value at runtime will be 1234567890123456800000000000000000000000000000000000000000
(lint/correctness/noPrecisionLoss)
packages/kit/src/provider/Container/PrimeLoginContainer/PrimeLoginContainer.tsx (2)
21-21: 🧹 Nitpick (assertive)
Address TODO comment about container renaming.
The comment suggests renaming to PrimeDialogContainer. Let's track this technical debt.
Would you like me to create an issue to track this renaming task?
37-39: 🧹 Nitpick (assertive)
Consider consolidating dialog refs.
Multiple similar refs could be combined into a single object for better maintainability.
- const emailDialogRef = useRef<IDialogInstance | undefined>(undefined); - const passwordDialogRef = useRef<IDialogInstance | undefined>(undefined); - const emailCodeDialogRef = useRef<IDialogInstance | undefined>(undefined); + const dialogRefs = useRef<Record<string, IDialogInstance | undefined>>({ + email: undefined, + password: undefined, + emailCode: undefined, + });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const dialogRefs = useRef<Record<string, IDialogInstance | undefined>>({ email: undefined, password: undefined, emailCode: undefined, });packages/kit-bg/src/dbs/simple/base/SimpleDb.ts (1)
40-41: 🧹 Nitpick (assertive)
Move prime property to maintain alphabetical order.
The prime property should be grouped with other similar properties to maintain the file's organization.
Move the property after line 92 to maintain alphabetical ordering with other properties.
packages/kit/src/views/Prime/components/PrimeLoginPasswordDialog/PrimeLoginPasswordDialog.tsx (3)
196-203: 🧹 Nitpick (assertive)
Remove commented-out code.
Dead code should be removed rather than commented out.
Remove the commented-out confirmPassword input section:
- {/* {isRegister ? ( - <Input - secureTextEntry - placeholder="Confirm Password" - value={confirmPassword} - onChangeText={setConfirmPassword} - /> - ) : null} */}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
85-111: 🛠️ Refactor suggestion
Enhance error handling in submit function.
The error handling could be more specific and user-friendly.
Add specific error handling:
try { const encodedPassword = await backgroundApiProxy.servicePassword.encodeSensitiveText({ text: formData.password, }); await backgroundApiProxy.servicePrime.ensurePrimeLoginValidPassword(encodedPassword); await backgroundApiProxy.servicePrime.resolvePrimeLoginPasswordDialog({ promiseId, password: encodedPassword, }); } catch (error) { options?.preventClose?.(); - throw error; + if (error instanceof ValidationError) { + form.setError('password', { message: 'Invalid password format' }); + } else if (error instanceof NetworkError) { + Toast.error({ title: 'Network error. Please try again.' }); + } else { + Toast.error({ title: 'An unexpected error occurred' }); + } + throw error; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const submit = useCallback( async (options: { preventClose?: () => void } = {}) => { await form.trigger(); if (!form.formState.isValid) { options?.preventClose?.(); return; } const formData = form.getValues(); try { const encodedPassword = await backgroundApiProxy.servicePassword.encodeSensitiveText({ text: formData.password, }); await backgroundApiProxy.servicePrime.ensurePrimeLoginValidPassword( encodedPassword, ); await backgroundApiProxy.servicePrime.resolvePrimeLoginPasswordDialog({ promiseId, password: encodedPassword, }); } catch (error) { options?.preventClose?.(); if (error instanceof ValidationError) { form.setError('password', { message: 'Invalid password format' }); } else if (error instanceof NetworkError) { Toast.error({ title: 'Network error. Please try again.' }); } else { Toast.error({ title: 'An unexpected error occurred' }); } throw error; } }, [form, promiseId], );
50-66: 🧹 Nitpick (assertive)
Password validation regex could be more secure.
The password validation checks for basic requirements but could be strengthened.
Consider adding these improvements:
- Maximum length limit
- Check for common password patterns
- Prevent password reuse
- Rate limit attempts
const isValidPassword = useCallback((password: string) => { + const MAX_LENGTH = 128; + if (password.length > MAX_LENGTH) { + return false; + } let minLength = true; let minNumberCharacter = true; let minLetterCharacter = true; let minSpecialCharacter = true;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const isValidPassword = useCallback((password: string) => { const MAX_LENGTH = 128; if (password.length > MAX_LENGTH) { return false; } let minLength = true; let minNumberCharacter = true; let minLetterCharacter = true; let minSpecialCharacter = true; if (password.length < 8) { minLength = false; } if (!/\d/.test(password)) { minNumberCharacter = false; } if (!/[a-zA-Z]/.test(password)) { minLetterCharacter = false; } // eslint-disable-next-line no-useless-escape if (!/[!@#$%^&*()_+\-=\[\]{};':"\\|,.<>/?`~]/.test(password)) { // further logic likely follows... } });apps/web-embed/pages/PageWebEmbedPrimePurchase.tsx (4)
274-281: 🛠️ Refactor suggestion
Use environment variables for test values.
Replace hardcoded test values with environment variables to prevent accidental exposure.
- if (process.env.NODE_ENV !== 'production') { - if (!primeUserId) { - primeUserId = 'did:privy:j32hasdfgg3288372'; - } - if (!primeUserEmail) { - primeUserEmail = '[email protected]'; - } - } + if (process.env.NODE_ENV !== 'production') { + if (!primeUserId) { + primeUserId = process.env.TEST_PRIME_USER_ID; + } + if (!primeUserEmail) { + primeUserEmail = process.env.TEST_PRIME_USER_EMAIL; + } + }
12-29: 🧹 Nitpick (assertive)
Add error handling for native toast requests.
The function should catch and handle potential failures when showing native toasts.
async function showNativeToast({ method, title, message, }: { method: 'success' | 'error' | 'info'; title: string; message?: string | undefined; }) { + try { await globalThis.$onekey.$private.request({ method: 'wallet_showToast', params: { method, title, message, }, }); + } catch (error) { + console.error('Failed to show native toast:', error); + } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.async function showNativeToast({ method, title, message, }: { method: 'success' | 'error' | 'info'; title: string; message?: string | undefined; }) { try { await globalThis.$onekey.$private.request({ method: 'wallet_showToast', params: { method, title, message, }, }); } catch (error) { console.error('Failed to show native toast:', error); } }
50-70: 🛠️ Refactor suggestion
Add accessibility attributes to the SVG spinner.
The SVG needs a title and aria-label for screen readers.
function Spinner() { return ( <svg width="24" height="24" viewBox="0 0 24 24" + role="img" + aria-label="Loading spinner" style={{ animation: 'spin 1s linear infinite', }} > + <title>Loading spinner</title> <path stroke="currentColor" strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d="M12 3v3m0 12v3M5.636 5.636l2.122 2.121m8.485 8.486 2.121 2.121M3 12.001h3m12 0h3M5.636 18.364l2.122-2.121m8.485-8.486 2.121-2.121" fill="none" /> </svg> ); }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.function Spinner() { return ( <svg width="24" height="24" viewBox="0 0 24 24" role="img" aria-label="Loading spinner" style={{ animation: 'spin 1s linear infinite', }} > <title>Loading spinner</title> <path stroke="currentColor" strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d="M12 3v3m0 12v3M5.636 5.636l2.122 2.121m8.485 8.486 2.121 2.121M3 12.001h3m12 0h3M5.636 18.364l2.122-2.121m8.485-8.486 2.121-2.121" fill="none" /> </svg> ); }🧰 Tools
🪛 Biome (1.9.4)
[error] 52-59: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
231-256:
⚠️ Potential issueImplement error handling in the purchase flow.
The purchase flow needs proper error handling and user feedback.
onClick={async () => { if (isLoading) { return; } try { setIsLoading(true); if (!selectedPackageId) { throw new Error('No package selected'); } const result = await purchasesSdk.purchasePackage({ userId: primeUserId, email: primeUserEmail, packageId: selectedPackageId, }); - // TODO toast error if (result) { await showNativeToast({ method: 'success', title: `Purchase success`, }); await closeNativeWebViewModal(); } + } catch (error) { + await showNativeToast({ + method: 'error', + title: 'Purchase failed', + message: error.message, + }); } finally { setIsLoading(false); } }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.onClick={async () => { if (isLoading) { return; } try { setIsLoading(true); if (!selectedPackageId) { throw new Error('No package selected'); } const result = await purchasesSdk.purchasePackage({ userId: primeUserId, email: primeUserEmail, packageId: selectedPackageId, }); if (result) { await showNativeToast({ method: 'success', title: `Purchase success`, }); await closeNativeWebViewModal(); } } catch (error) { await showNativeToast({ method: 'error', title: 'Purchase failed', message: error.message, }); } finally { setIsLoading(false); } }}packages/kit-bg/src/services/ServiceCustomRpc.ts (1)
242-245: 🛠️ Refactor suggestion
Add proper error handling for network fetch.
Using void to ignore the promise could hide important errors. Consider logging or handling errors more explicitly.
- void this.fetchNetworkFromServer().catch((error) => { - defaultLogger.account.wallet.getServerNetworksError(error); - }); + this.fetchNetworkFromServer().catch((error) => { + defaultLogger.account.wallet.getServerNetworksError(error); + // Consider emitting an event or updating state to show fetch failure + appEventBus.emit(EAppEventBusNames.NetworkFetchFailed, error); + });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.this.fetchNetworkFromServer().catch((error) => { defaultLogger.account.wallet.getServerNetworksError(error); // Consider emitting an event or updating state to show fetch failure appEventBus.emit(EAppEventBusNames.NetworkFetchFailed, error); }); }packages/kit/src/views/Prime/pages/PrimeDashboard/PrimeDashboard.tsx (2)
93-107:
⚠️ Potential issueFix hardcoded wait time and empty URL.
The function uses a hardcoded wait time and an empty URL, which could cause reliability issues.
const purchaseByWebview = useCallback(async () => { navigation.popStack(); - await timerUtils.wait(1000); + // Consider using a navigation event listener instead of arbitrary wait + await new Promise(resolve => navigation.addListener('transitionEnd', resolve)); openUrlUtils.openUrlByWebviewPro({ - url: '', + url: process.env.PRIME_PURCHASE_URL || 'https://prime.onekey.so/purchase', title: 'WebView', isWebEmbed: true, hashRoutePath: EWebEmbedRoutePath.primePurchase, hashRouteQueryParams: { primeUserId: user?.privyUserId || '', primeUserEmail: user?.email || '', }, }); }, [navigation, user?.privyUserId, user?.email]);
238-312:
⚠️ Potential issueRemove or hide debug buttons in production.
Debug buttons expose sensitive operations and use console.log. These should be hidden in production.
+const DebugButtons = () => { + if (process.env.NODE_ENV === 'production') { + return null; + } + return ( <XStack flexWrap="wrap"> <Button onPress={() => { void logout(); }}>Logout</Button> {/* ... other debug buttons ... */} </XStack> + ); +}; // In the render method: -<XStack flexWrap="wrap"> - {/* debug buttons */} -</XStack> +<DebugButtons />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.// Add the DebugButtons component const DebugButtons = () => { if (process.env.NODE_ENV === 'production') { return null; } return ( <XStack flexWrap="wrap"> <Button onPress={() => { void logout(); }} > Logout </Button> <Button onPress={() => { void getAccessToken().then(showDebugMessageByDialog); }} > Get Access Token </Button> <Button onPress={() => { showDebugMessageByDialog({ ready: isReady, authenticated, }); }} > User Info </Button> <Button onPress={() => { // }} > shouldShowConfirmButton={shouldShowConfirmButton.toString()} </Button> <Button onPress={() => { void getCustomerInfo().then(showDebugMessageByDialog); }} > CustomerInfo </Button> <Button onPress={() => { void fetchPrimeUserInfo().then(showDebugMessageByDialog); }} > ServerPrimeUserInfo </Button> <Button onPress={() => { void getPaywallPackagesNative?.().then(showDebugMessageByDialog); void getPaywallPackagesWeb?.().then(showDebugMessageByDialog); }} > PaywallPackages </Button> <Button onPress={() => { void backgroundApiProxy.servicePrime .apiGetPrimeUserDevices() .then(console.log); }} > UserDevices </Button> <Button onPress={() => { navigation.pushFullModal(EModalRoutes.PrimeModal, { screen: EPrimePages.PrimeDeviceLimit, }); }} > DeviceLimit </Button> </XStack> ); }; // In your component's render method (or return block), replace the old debug buttons block: return ( <> {/* other components */} <DebugButtons /> {/* other components */} </> );development/webpack/webpack.base.config.js (1)
106-116: 🧹 Nitpick (assertive)
Use environment variables for babel loader configuration.
Consider making babel loader options configurable through environment variables.
const babelLoaderOption = { babelrc: false, configFile: true, sourceType: 'unambiguous', root: basePath, - compact: !isDev, - sourceMaps: isDev, - inputSourceMap: isDev, + compact: process.env.BABEL_COMPACT !== 'false' && !isDev, + sourceMaps: process.env.BABEL_SOURCEMAPS === 'true' || isDev, + inputSourceMap: process.env.BABEL_SOURCEMAPS === 'true' || isDev, cacheCompression: false, cacheDirectory: path.resolve(basePath, 'node_modules/.cache/babel-loader'), };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const babelLoaderOption = { babelrc: false, configFile: true, sourceType: 'unambiguous', root: basePath, compact: process.env.BABEL_COMPACT !== 'false' && !isDev, sourceMaps: process.env.BABEL_SOURCEMAPS === 'true' || isDev, inputSourceMap: process.env.BABEL_SOURCEMAPS === 'true' || isDev, cacheCompression: false, cacheDirectory: path.resolve(basePath, 'node_modules/.cache/babel-loader'), };.eslintrc.js (1)
1-2: 💡 Verification agent
🧩 Analysis chain
Verify the impact of disabling force-async-bg-api rule
The force-async-bg-api rule and its import are commented out with TODO comments. This could lead to inconsistent background method handling.
Run this script to find methods that would be affected:
Also applies to: 11-11
🏁 Script executed:
#!/bin/bash # Find methods with @backgroundMethod decorator that might need async ast-grep --pattern $'@backgroundMethod() $$$ $method() { $$$ }'Length of output: 66
🏁 Script executed:
#!/bin/bash echo "Searching for any instance of force-async-bg-api:" rg 'force-async-bg-api' echo "------------------------------" echo "Searching for occurrences of '@backgroundMethod':" rg '@backgroundMethod'Length of output: 66105
Impact of Disabling 'force-async-bg-api' ESLint Rule
Our investigation confirms that:
- The custom ESLint rule (force-async-bg-api) is defined in development/lint/eslint-rule-force-async-bg-api.js but is commented out in .eslintrc.js.
- Numerous files, particularly in the packages/kit-bg directory, use the @backgroundMethod decorator.
This setup means that any methods marked with @backgroundMethod aren’t being checked for an async declaration. Without the lint rule enforcing async usage, there’s a risk that a background method may be implemented without the proper asynchronous handling, potentially leading to inconsistencies or subtle bugs.
We recommend reviewing those methods manually or re-enabling the rule to ensure robust async behavior.
packages/components/src/composite/Dialog/index.tsx (1)
518-544: 🧹 Nitpick (assertive)
Consider adding error handling for clipboard operations
The clipboard operation in dialogDebugMessage should handle potential failures.
- onPress={async () => { - await setStringAsync(dataContent); - Toast.success({ - title: 'Copied', - }); + onPress={async () => { + try { + await setStringAsync(dataContent); + Toast.success({ + title: 'Copied', + }); + } catch (error) { + Toast.error({ + title: 'Failed to copy', + }); + } + }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const dialogDebugMessage = ( props: IDialogShowProps & { debugMessage: any }, ) => { const dataContent = JSON.stringify(props.debugMessage, null, 2); console.log('dialogDebugMessage:', dataContent); return dialogShow({ title: 'DebugMessage', showFooter: false, showConfirmButton: false, showCancelButton: false, renderContent: ( <ScrollView maxHeight="$48" nestedScrollEnabled> <SizableText onPress={async () => { try { await setStringAsync(dataContent); Toast.success({ title: 'Copied', }); } catch (error) { Toast.error({ title: 'Failed to copy', }); } }} > {dataContent} </SizableText> </ScrollView> ), ...props, }); };packages/shared/src/errors/errors/appErrors.ts (2)
120-129: 🧹 Nitpick (assertive)
Add className override for consistency.
Similar to the previous error class, consider adding className override.
export class OneKeyErrorPrimeLoginExceedDeviceLimit extends OneKeyError { constructor(props?: IOneKeyError | string) { super( normalizeErrorProps(props, { defaultMessage: 'Prime exceed device limit', defaultAutoToast: true, }), ); } + override className = EOneKeyErrorClassNames.OneKeyErrorPrimeLoginExceedDeviceLimit; }
109-118: 🧹 Nitpick (assertive)
Add className override for consistency.
Other error classes in this file override className. Consider adding it here for consistency.
export class OneKeyErrorPrimeLoginInvalidToken extends OneKeyError { constructor(props?: IOneKeyError | string) { super( normalizeErrorProps(props, { defaultMessage: 'Prime login invalid, please login again', defaultAutoToast: true, }), ); } + override className = EOneKeyErrorClassNames.OneKeyErrorPrimeLoginInvalidToken; }packages/kit/src/views/Onboarding/pages/ConnectHardwareWallet/ConnectYourDevice.tsx (1)
1-1: 🧹 Nitpick (assertive)
Consider extracting nested components to separate files.
The ESLint rule
react/no-unstable-nested-componentsis disabled. This rule prevents performance issues and improves code organization. Instead of disabling it:
- Move
DeviceListItem,ConnectByQrCode, and other nested components to separate files- Import them where needed
packages/kit/src/components/Captcha/Captcha.tsx (1)
1-3: 🧹 Nitpick (assertive)
Minimal Captcha Component.
The Captcha component currently returns an empty fragment. Consider adding implementation details or a placeholder comment if further work is planned..github/workflows/unittest.yml (1)
73-73: 🧹 Nitpick (assertive)
Remove Extra Blank Line.
Static analysis detected an extra blank line here. Please remove it to maintain a clean YAML file.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 73-73: too many blank lines
(1 > 0) (empty-lines)
.github/workflows/release-android-debug.yml (2)
10-10: 🧹 Nitpick (assertive)
Minor Formatting Adjustment.
Line 10 shows a formatting change (likely a newline tweak). Please confirm that this change was intentional.
84-84: 🧹 Nitpick (assertive)
Formatting Check.
Line 84 appears as an isolated change. Verify if this line was added for spacing or if any content was meant to be updated..github/workflows/release-android.yml (2)
15-15: 🧹 Nitpick (assertive)
Workflow Dispatch Formatting.
Line 15 has a minor formatting change. Please verify if this blank/update is intentional.
107-107: 🧹 Nitpick (assertive)
Formatting Check Post-EAS Build.
Line 107 appears to be a formatting adjustment. Confirm if this whitespace change is deliberate..github/workflows/release-ios.yml (1)
91-94: 🧹 Nitpick (assertive)
Sentry Properties Write.
There’s a minor change at line 94—possibly a trailing newline or spacing adjustment. Verify if any additional sentry configuration was intended here..github/workflows/release-desktop-snap.yml (2)
12-12: 🧹 Nitpick (assertive)
Remove Trailing Whitespace.
Extra spaces on this line should be removed to pass YAMLlint.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 12-12: trailing spaces
(trailing-spaces)
25-26: 🧹 Nitpick (assertive)
Clean Up Extra Spaces.
There are trailing spaces in these lines. Removing them will improve clarity.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 25-25: trailing spaces
(trailing-spaces)
[error] 26-26: trailing spaces
(trailing-spaces)
.github/workflows/release-desktop.yml (1)
25-25: 🧹 Nitpick (assertive)
Remove Trailing Whitespace.
Please eliminate the extra spaces on this line.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 25-25: trailing spaces
(trailing-spaces)
.github/actions/shared-env/action.yml (3)
4-50: 🧹 Nitpick (assertive)
Review Input Parameters.
All critical inputs are defined. Consider adding a definition for "is-split-bundle" since it is used later.
151-151: 🧹 Nitpick (assertive)
Remove Trailing Whitespace.
Eliminate extra spaces on this line for cleaner formatting.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 151-151: trailing spaces
(trailing-spaces)
174-174: 🧹 Nitpick (assertive)
Reduce Extra Blank Lines.
Remove the superfluous blank line to improve readability.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 174-174: too many blank lines
(2 > 0) (empty-lines)
There was a problem hiding this comment.
Actionable comments posted: 10
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
packages/kit/src/components/WebViewWebEmbed/index.tsx(5 hunks)packages/kit/src/views/Prime/components/PrivyProvider.native.tsx(1 hunks)packages/kit/src/views/Prime/components/PrivyProvider.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: unittest (20.x)
- GitHub Check: lint (20.x)
🔇 Additional comments (15)
packages/kit/src/views/Prime/components/PrivyProvider.native.tsx (1)
1-7: Clean and focused imports!The imports are well-organized and purposeful.
packages/kit/src/views/Prime/components/PrivyProvider.tsx (1)
26-29: Document login method restrictions.The code comments out several login methods. Add a comment explaining why only email login is enabled.
Are the other login methods intentionally disabled? Please confirm if this is a temporary or permanent restriction.
packages/kit/src/components/WebViewWebEmbed/index.tsx (13)
5-13: Looks good.
These imports accurately add new hooks and route enums. No issues spotted.
25-31: Clean extension of props.
Adding hashRoutePath and hashRouteQueryParams improves navigation. Implementation looks consistent.
40-42: Neat usage of new hooks.
Good job fetching theme and locale variants here.
44-44: State variable initialization is fine.
Storing an empty string as the default for the key is logical.
47-57: Method to fetch API key is clear.
You fetch dev settings, switch keys if sandbox is enabled, and handle missing keys. All good.
59-60: Side-effect is correct.
You only call getApiKey once on mount. Simple and effective.
62-73: Smart memo usage.
You wait for theme, locale, and the API key before populating webEmbedAppSettings. Looks structured.
76-79: Conditional remoteUrl logic is sound.
In dev or when devSettingsPersistAtom.enabled, you fall back to config.url. Handling is neat.Also applies to: 85-85
106-109: Early return prevents errors.
Returning null if webEmbedAppSettings is absent is a good defensive pattern.
133-141: Dependency array is correct.
All relevant dependencies are included. Looks good.
143-161: Bridge integration is well-structured.
You handle platform checks and connect backgroundApiProxy properly.
225-230: Singleton view is neatly composed.
No issues with pass-through props or usage.
123-130: Embed caution with sensitive keys.
Embedding the API key in window object is okay if it’s not truly private. Verify it’s safe.Below is a script to search for references to these environment variables. Assess if they're exposed externally.
✅ Verification successful
🏁 Script executed:
#!/bin/bash rg "REVENUECAT_API_KEY_WEB" -A 5Length of output: 4586
API key exposure is intentional and safe.
The grep results confirm that REVENUECAT_API_KEY_WEB is listed in the file (development/envExposedToClient.js) that defines which environment variables are exposed to the client. This indicates that the API key is not treated as a truly private secret and is safe to embed in the window object.
packages/kit/src/views/Prime/components/PrivyProvider.native.tsx
Outdated
Show resolved
Hide resolved
packages/kit/src/views/Prime/components/PrimeLoginEmailCodeDialog/PrimeLoginEmailCodeDialog.tsx
Outdated
Show resolved
Hide resolved
packages/kit/src/views/Prime/components/PrimeLoginEmailCodeDialog/PrimeLoginEmailCodeDialog.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 24
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
apps/desktop/src-electron/app.ts(1 hunks)apps/web-embed/pages/PageWebEmbedPrimePurchase.tsx(1 hunks)development/spellCheckerSkipWords.js(3 hunks)packages/components/src/forms/Slider/index.native.tsx(1 hunks)packages/kit-bg/src/services/ServiceCloudBackup/index.ts(1 hunks)packages/kit-bg/src/services/ServicePassword/index.ts(1 hunks)packages/kit-bg/src/services/ServicePrime/ServicePrime.tsx(1 hunks)packages/kit/src/components/Password/components/PasswordVerify.tsx(1 hunks)packages/kit/src/views/Prime/components/PrimeLoginEmailCodeDialog/PrimeLoginEmailCodeDialog.tsx(1 hunks)packages/kit/src/views/Prime/components/PrimeLoginEmailCodeDialogV2/PrimeLoginEmailCodeDialogV2.tsx(1 hunks)packages/shared/src/utils/debug/dbPerfMonitor.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/web-embed/pages/PageWebEmbedPrimePurchase.tsx
[error] 52-59: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
[error] 177-178: The elements with the following roles can be changed to the following elements:
For examples and more information, see WAI-ARIA Roles
(lint/a11y/useSemanticElements)
packages/kit-bg/src/services/ServicePrime/ServicePrime.tsx
[error] 404-413: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
[error] 451-464: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
[error] 499-512: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: lint (20.x)
🔇 Additional comments (9)
packages/shared/src/utils/debug/dbPerfMonitor.ts (1)
183-183: Type Update is Spot-On!Switching to ReturnType improves type inference and ensures compatibility across both Node and browser environments. No issues detected.
packages/components/src/forms/Slider/index.native.tsx (1)
26-26: Type change looks good!Using
ReturnType<typeof setTimeout>instead ofNodeJS.Timeoutprovides better type safety and platform compatibility.packages/kit/src/components/Password/components/PasswordVerify.tsx (1)
86-86: Type change looks good!Using
ReturnType<typeof setTimeout> | nullinstead ofNodeJS.Timeout | nullprovides better type safety while maintaining null union type.packages/kit-bg/src/services/ServicePassword/index.ts (1)
70-70: Type changes look good!The updates to timeout and interval types provide better type safety:
cachedPasswordTimeOutObject: Now usesReturnType<typeof setTimeout>passwordPromptTimeout: Now usesReturnType<typeof setTimeout>extCheckLockStatusTimer: Now usesReturnType<typeof setInterval>Also applies to: 76-76, 80-80
packages/kit-bg/src/services/ServiceCloudBackup/index.ts (1)
668-668: Timer type updated for cross-platform consistency.
Updating the timer property to ReturnType improves type safety across environments. This change aligns with similar updates in our codebase and reduces potential mismatches between Node.js and browser timer types.packages/kit-bg/src/services/ServicePrime/ServicePrime.tsx (2)
339-341: 🧹 Nitpick (assertive)Implement CAPTCHA verification.
The TODO comment about CAPTCHA verification needs implementation. This is important for security.
Would you like me to help implement the CAPTCHA verification logic or create an issue to track this task?
391-391: 🧹 Nitpick (assertive)Implement password management dialogs.
The TODO comments for forget password and change password dialogs need implementation.
Would you like me to help implement these password management dialogs or create issues to track these tasks?
Also applies to: 398-399
packages/kit/src/views/Prime/components/PrimeLoginEmailCodeDialog/PrimeLoginEmailCodeDialog.tsx (2)
45-57: LGTM! Clean timer implementation.The countdown timer implementation is correct with proper cleanup and type definition.
59-61: LGTM! Proper initial code sending.The effect correctly triggers the verification code sending on component mount.
...s/kit/src/views/Prime/components/PrimeLoginEmailCodeDialogV2/PrimeLoginEmailCodeDialogV2.tsx
Show resolved
Hide resolved
...s/kit/src/views/Prime/components/PrimeLoginEmailCodeDialogV2/PrimeLoginEmailCodeDialogV2.tsx
Show resolved
Hide resolved
...s/kit/src/views/Prime/components/PrimeLoginEmailCodeDialogV2/PrimeLoginEmailCodeDialogV2.tsx
Show resolved
Hide resolved
packages/kit/src/views/Prime/components/PrimeLoginEmailCodeDialog/PrimeLoginEmailCodeDialog.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 16
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
apps/desktop/src-electron/app.ts(1 hunks)packages/kit-bg/src/services/ServicePrime/ServicePrime.tsx(1 hunks)packages/kit/src/views/Prime/components/PrimeLoginEmailCodeDialog/PrimeLoginEmailCodeDialog.tsx(1 hunks)packages/kit/src/views/Prime/components/PrimeLoginEmailCodeDialogV2/PrimeLoginEmailCodeDialogV2.tsx(1 hunks)packages/kit/src/views/Prime/hooks/usePrivyAuthSyncToAtom.ts(1 hunks)packages/kit/src/views/Prime/hooks/usePrivyUniversalV2/usePrivyUniversalV2.native.ts(1 hunks)packages/kit/src/views/Prime/hooks/usePrivyUniversalV2/usePrivyUniversalV2.ts(1 hunks)packages/kit/src/views/Prime/hooks/usePrivyUniversalV2/usePrivyUniversalV2Types.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/kit-bg/src/services/ServicePrime/ServicePrime.tsx
[error] 399-408: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
[error] 446-459: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
[error] 494-507: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: lint (20.x)
- GitHub Check: unittest (20.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (30)
apps/desktop/src-electron/app.ts (1)
79-79: Type Update Confirmed.
The variable is now typed as ReturnType, which correctly matches its usage for setInterval. This aligns with previous feedback.packages/kit/src/views/Prime/hooks/usePrivyUniversalV2/usePrivyUniversalV2Types.ts (1)
21-22: Clarify email parameter requirement.The email parameter in
loginWithCodeis marked optional, but it might be required if not in the correct state. Consider adding runtime validation or making it required.Could you confirm if the email parameter is truly optional in all cases?
packages/kit/src/views/Prime/hooks/usePrivyUniversalV2/usePrivyUniversalV2.native.ts (2)
1-5: Clean imports - looking good!The imports are well-organized and each serves a clear purpose.
24-54: 🛠️ Refactor suggestionEnhance error handling in async operations
The spread operator usage could mask errors. Also, empty string fallbacks might cause issues.
- id: user?.id || '', - email: userEmail || '', + id: user?.id ?? '', + email: userEmail ?? '',Also, consider adding error handling in async functions:
sendCode: async (...sendCodeArgs) => { - await sendCode(...sendCodeArgs); + try { + await sendCode(...sendCodeArgs); + } catch (error) { + onError?.(error); + throw error; + } },packages/kit/src/views/Prime/components/PrimeLoginEmailCodeDialogV2/PrimeLoginEmailCodeDialogV2.tsx (2)
16-16: Move COUNTDOWN_TIME to a constants file.Move this constant to a shared constants file. Other components might need the same countdown duration.
33-68: LGTM! Clean implementation of the timer and resend logic.The code handles edge cases well and uses setTimeout for accurate timing.
packages/kit/src/views/Prime/hooks/usePrivyUniversalV2/usePrivyUniversalV2.ts (3)
1-4: Clean imports with good type separation!The imports are focused and well-organized, with clear separation between external and internal dependencies.
20-25: Avoid spreading unknown arguments to async functions.The spread operator with async functions can lead to unexpected behavior if the arguments don't match.
- sendCode: async (...sendCodeArgs) => { - await sendCode(...sendCodeArgs); + sendCode: async (email: string) => { + await sendCode(email); }, - loginWithCode: async (...loginWithCodeArgs) => { - await loginWithCode(...loginWithCodeArgs); + loginWithCode: async (code: string) => { + await loginWithCode(code); },
32-37: 🛠️ Refactor suggestionAdd null checks for user properties.
The current user object transformation could throw if user is null.
user: authenticated ? { - id: user?.id || '', - email: user?.email?.address || '', + id: user?.id ?? '', + email: user?.email?.address ?? '', }Likely invalid or redundant comment.
packages/kit/src/views/Prime/components/PrimeLoginEmailCodeDialog/PrimeLoginEmailCodeDialog.tsx (13)
1-2: No issues found.
The imports look clean and straightforward.
3-15: No issues found.
These UI imports and background proxy references are consistent.
16-16: Make the constant name more descriptive.
Replace COUNTDOWN_TIME with a clearer naming, for example RESEND_CODE_COUNTDOWN_SECONDS.
18-22: Function signature looks good.
No suggestions here.
23-28: Consolidate duplicate state variables.
You keep both “code” and “verificationCode.” Merging them reduces confusion and potential errors.
29-43: Add validation checks before sending the code.
Guard against empty email and verifyUUID before making the API call.
45-57: Countdown logic looks correct.
The timer clears properly to avoid memory leaks.
59-61: Auto-triggering code send is fine.
This ensures the code dispatch on mount.
63-67: Internationalize button text.
Avoid hardcoded Chinese strings. Use translation keys.
69-100: General UI structure is fine.
No issues spotted in the layout or button usage.
102-113: Remove the verification code from Toast.
Showing codes in Toast is a security concern.
116-132: Add loading feedback on confirmation.
Disable or show a spinner to prevent multiple submissions while awaiting resolution.
135-135: No issues.
The closing of the component is concise.packages/kit-bg/src/services/ServicePrime/ServicePrime.tsx (8)
53-60: **Use cookies instead of simpleDb **
This TODO repeats a past suggestion. Storing tokens in cookies is more secure.
110-110: Verify 404 endpoint
“TODO 404 not found.” Ensure the endpoint path is correct or handle the possible 404 error.
184-228: **Remove or clean out old commented code **
These lines are marked deprecated. A previous reviewer suggested fully removing them.
290-293: **Localize error message **
Use i18n for "Invalid email" to keep error handling consistent.
370-389: **Remove console.log **
A past reviewer already suggested removing console logs in production code.
399-408: **Avoid async in Promise executor **
This is the same anti-pattern mentioned before.🧰 Tools
🪛 Biome (1.9.4)
[error] 399-408: Promise executor functions should not be
async.(lint/suspicious/noAsyncPromiseExecutor)
446-459: **Avoid async in Promise executor **
Same fix needed to remove the async from the executor.🧰 Tools
🪛 Biome (1.9.4)
[error] 446-459: Promise executor functions should not be
async.(lint/suspicious/noAsyncPromiseExecutor)
494-507: **Avoid async in Promise executor **
Same issue as above.🧰 Tools
🪛 Biome (1.9.4)
[error] 494-507: Promise executor functions should not be
async.(lint/suspicious/noAsyncPromiseExecutor)
packages/kit/src/views/Prime/hooks/usePrivyUniversalV2/usePrivyUniversalV2Types.ts
Show resolved
Hide resolved
packages/kit/src/views/Prime/hooks/usePrivyUniversalV2/usePrivyUniversalV2Types.ts
Show resolved
Hide resolved
packages/kit/src/views/Prime/hooks/usePrivyUniversalV2/usePrivyUniversalV2.native.ts
Show resolved
Hide resolved
packages/kit/src/views/Prime/hooks/usePrivyUniversalV2/usePrivyUniversalV2.native.ts
Show resolved
Hide resolved
...s/kit/src/views/Prime/components/PrimeLoginEmailCodeDialogV2/PrimeLoginEmailCodeDialogV2.tsx
Show resolved
Hide resolved
packages/kit/src/views/Prime/hooks/usePrivyUniversalV2/usePrivyUniversalV2.ts
Show resolved
Hide resolved
packages/kit/src/views/Prime/hooks/usePrivyUniversalV2/usePrivyUniversalV2.ts
Show resolved
Hide resolved
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/kit-bg/src/providers/ProviderApiPrivate.ts(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: lint (20.x)
- GitHub Check: unittest (20.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
packages/kit-bg/src/providers/ProviderApiPrivate.ts (2)
11-11: LGTM! Clean import additions.The new imports support the toast notification feature.
Also applies to: 19-19, 31-34
505-510: Great logging enhancement!The detailed JSON logging will make debugging web embed API issues easier.
Summary by CodeRabbit
New Features
Refactor
Documentation
Chores