Nijil / Replace useDtraderQuery with useQuery in AppV2 #384
Conversation
🤖 AI Code Analysis Results📝 Manual Coding⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜⬜ 0% 0 of 29,242 characters (0%) in newly added lines are AI-generated 📊 Analysis Summary
📋 Per-File Breakdown (New Lines Only)
📝 This PR contains traditional hand-written code. 📊 Raw Data (for dashboard){
"timestamp": "2025-10-22T11:47:55.804Z",
"repository": "deriv-com/derivatives-trader",
"branch": "unknown",
"author": "unknown",
"summary": {
"totalCharacters": 29242,
"aiCharacters": 0,
"percentage": 0
},
"files": [
{
"name": "packages/trader/src/AppV2/Components/ClosedMarketMessage/closed-market-message.tsx",
"totalCharacters": 95,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": "import { toMoment, useIsMounted, WS } from '@deriv/shared';\n }, [symbol, is_market_closed]);"
},
{
"name": "packages/trader/src/AppV2/Components/MarketSelector/market-selector.scss",
"totalCharacters": 78,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": " &__skeleton {\n padding: 0 var(--semantic-spacing-general-sm);\n }"
},
{
"name": "packages/trader/src/AppV2/Components/MarketSelector/market-selector.tsx",
"totalCharacters": 275,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": " const { activeSymbols, isLoading } = useActiveSymbols();\n if (!currentSymbol && !isLoading) {\n return (\n <div className='market-selector__skeleton'>\n <Skeleton.Square width={200} height={42} rounded />\n </div>\n );"
},
{
"name": "packages/trader/src/AppV2/Components/TradeParameters/Duration/__tests__/duration.spec.tsx",
"totalCharacters": 112,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": "jest.mock('../day', () => ({\n __esModule: true,\n default: jest.fn(() => <div>Mocked DayInput</div>),\n}));\n"
},
{
"name": "packages/trader/src/AppV2/Components/TradeParameters/Duration/__tests__/duration_container.spec.tsx",
"totalCharacters": 320,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": "import { render, screen } from '@testing-library/react';\njest.mock('../day', () => ({\n __esModule: true,\n default: jest.fn(() => <div>Mocked DayInput</div>),\n}));\n\n it('should render DayInput component when days unit is selected', () => {\n expect(screen.getByText('Mocked DayInput')).toBeInTheDocument();"
},
{
"name": "packages/trader/src/AppV2/Components/TradeParameters/Duration/day.tsx",
"totalCharacters": 1617,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": "import { useInvalidateQuery } from '@deriv/api';\nimport { Localize } from '@deriv-com/translations';\nimport { useProposal } from 'AppV2/Hooks/useProposal';\nimport { getClosestTimeToCurrentGMT, getDatePickerStartDate } from 'AppV2/Utils/trade-params-utils';\n ...(barrier_1 && !is_turbos && !barrier_value ? { barrier_1: Math.round(tick_data?.quote as number) } : {}),\n const { data: response, error: queryError } = useProposal({\n proposal_request_values: new_values,\n contract_..."
},
{
"name": "packages/trader/src/AppV2/Components/TradeParameters/PayoutPerPoint/__tests__/payout-per-point.spec.tsx",
"totalCharacters": 691,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": "jest.mock('../payout-per-point-wheel', () => ({\n __esModule: true,\n default: jest.fn(({ barrier, onPayoutPerPointSelect, onClose, payout_per_point_list }) => (\n <div>\n <p>WheelPicker</p>\n <ul>\n {payout_per_point_list.map(({ value }: { value: string }) => (\n <li key={value}>\n <button onClick={() => onPayoutPerPointSelect(value)}>{value}</button>\n </li>\n ))}\n </u..."
},
{
"name": "packages/trader/src/AppV2/Components/TradeParameters/PayoutPerPoint/payout-per-point-wheel.tsx",
"totalCharacters": 635,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": "import { Localize } from '@deriv-com/translations';\nimport { useProposal } from 'AppV2/Hooks/useProposal';\n\n // Sending proposal without subscription to get a new barrier value\n const { data: response } = useProposal({\n proposal_request_values: new_values,\n contract_type: Object.keys(trade_types)[0],\n is_enabled: is_open,\n if (response) {\n const { proposal } = response;\n const { barrier_spot_distance } = prop..."
},
{
"name": "packages/trader/src/AppV2/Components/TradeParameters/RiskManagement/__tests__/take-profit-and-stop-loss-container.spec.tsx",
"totalCharacters": 282,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": " send: jest.fn(),\n authorized: {\n send: jest.fn(),\n },\njest.mock('AppV2/Hooks/useProposal', () => ({\n useProposal: jest.fn(() => ({\n data: {\n proposal: {},\n },\n error: null,\n isFetching: false,\n })),\n}));\n"
},
{
"name": "packages/trader/src/AppV2/Components/TradeParameters/RiskManagement/__tests__/take-profit-and-stop-loss-input.spec.tsx",
"totalCharacters": 127,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": "jest.mock('AppV2/Hooks/useProposal', () => ({\n useProposal: jest.fn(() => ({\n error: null,\n isFetching: false,"
},
{
"name": "packages/trader/src/AppV2/Components/TradeParameters/RiskManagement/take-profit-and-stop-loss-input.tsx",
"totalCharacters": 1127,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": "import { useProposal } from 'AppV2/Hooks/useProposal';\nimport { focusAndOpenKeyboard } from 'AppV2/Utils/trade-params-utils';\n const { data: response, error: queryError } = useProposal({\n proposal_request_values: new_values,\n contract_type: Object.keys(trade_types)[0],\n is_enabled,\n // Exclude the opposite field when validating tp/sl independently\n should_skip_validation: is_take_profit_input ? 'stop_loss' : 'take_profit',\n if (queryError) {\n ..."
},
{
"name": "packages/trader/src/AppV2/Components/TradeParameters/Stake/__tests__/stake.spec.tsx",
"totalCharacters": 579,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": "\n\njest.mock('AppV2/Hooks/useProposal', () => ({\n useProposal: jest.fn(() => ({\n error: null,\n isFetching: false,\n [CONTRACT_TYPES.CALL]: { stake: { max: '50000.00', min: '0.35' } },\n [CONTRACT_TYPES.PUT]: { stake: { max: '50000.00', min: '0.35' } },\n default_mock_store.modules.trade.validation_params = {\n CALL: { stake: { max: '50000.00', min: '0.35' } },\n\n // Verify the acceptable range message is sh..."
},
{
"name": "packages/trader/src/AppV2/Components/TradeParameters/Stake/stake-input.tsx",
"totalCharacters": 2680,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": "import { TPriceProposalResponse, TSocketError } from '@deriv/api';\nimport { useProposal } from 'AppV2/Hooks/useProposal';\n is_first_payout_exceeded: !!first_payout_error && first_contract_payout > Number(max_payout),\n is_second_payout_exceeded: !!second_payout_error && second_contract_payout > Number(max_payout),\n const {\n data: response_1,\n error: error_1,\n isFetching: is_fetching_1,\n } = useProposal({\n const {\n data: response_2,\n ..."
},
{
"name": "packages/trader/src/AppV2/Containers/Trade/__tests__/trade.spec.tsx",
"totalCharacters": 40,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": " ...jest.requireActual('@deriv/api'),"
},
{
"name": "packages/trader/src/AppV2/Hooks/__tests__/useActiveSymbols.spec.tsx",
"totalCharacters": 2371,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": "import { useQuery } from '@deriv/api';\nimport { CONTRACT_TYPES, TRADE_TYPES } from '@deriv/shared';\njest.mock('@deriv/api', () => ({\n ...jest.requireActual('@deriv/api'),\n useQuery: jest.fn(() => ({\n data: {\n active_symbols: not_logged_in_active_symbols,\n },\n error: null,\n isLoading: false,\n })),\n}));\n\n jest.clearAllMocks();\n (useQuery as jest.Mock).mockReturnValue({\n data: { active_symbols: not_logged_in_active_symbols },\n..."
},
{
"name": "packages/trader/src/AppV2/Hooks/__tests__/useContractsFor.spec.tsx",
"totalCharacters": 3234,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": "import { useQuery } from '@deriv/api';\nimport { cloneObject, getContractCategoriesConfig, getContractTypesConfig } from '@deriv/shared';\n\njest.mock('@deriv/api', () => ({\n ...jest.requireActual('@deriv/api'),\n useQuery: jest.fn(() => ({\n data: null,\n error: null,\n isLoading: false,\n })),\n}));\n jest.clearAllMocks();\n (useQuery as jest.Mock).mockReturnValue({\n data: {\n contracts_for: {\n available: [\n ..."
},
{
"name": "packages/trader/src/AppV2/Hooks/__tests__/useProposal.spec.tsx",
"totalCharacters": 10688,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": "import React from 'react';\n\nimport { APIProvider } from '@deriv/api';\nimport { QueryClient, QueryClientProvider } from '@tanstack/react-query';\nimport { renderHook } from '@testing-library/react-hooks';\n\nimport { getProposalRequestObject } from 'AppV2/Utils/trade-params-utils';\nimport { useTraderStore } from 'Stores/useTraderStores';\nimport type { TTradeStore } from 'Types';\n\nimport { useProposal } from '../useProposal';\n\njest.mock('Stores/useTraderStores', () => ({\n useTraderStore: jest.fn()..."
},
{
"name": "packages/trader/src/AppV2/Hooks/useActiveSymbols.ts",
"totalCharacters": 1799,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": "import { useCallback, useEffect } from 'react';\nimport { TActiveSymbolsRequest, useQuery } from '@deriv/api';\nimport { CONTRACT_TYPES, getContractTypesConfig } from '@deriv/shared';\ntype TContractTypesList = NonNullable<TActiveSymbolsRequest['contract_type']>;\n// Cache configuration for active symbols query\nconst ACTIVE_SYMBOLS_CACHE_CONFIG = {\n CACHE_TIME: 10 * 60 * 1000, // 10 minutes - keep in cache even if unused\n} as const;\n/**\n * Hook to fetch and manage active symbols for trading\n */\n ..."
},
{
"name": "packages/trader/src/AppV2/Hooks/useContractsFor.ts",
"totalCharacters": 330,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": "import { useQuery } from '@deriv/api';\n isLoading,\n } = useQuery('contracts_for', {\n payload: {\n options: {\n refetchOnWindowFocus: false, // Don't refetch when window regains focus\n },\n });\n const is_fetching_ref = useRef(isLoading);\n is_fetching_ref.current = isLoading;"
},
{
"name": "packages/trader/src/AppV2/Hooks/useProposal.tsx",
"totalCharacters": 1749,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": "import React from 'react';\n\nimport { TPriceProposalRequest, useQuery } from '@deriv/api';\n\nimport { getProposalRequestObject } from 'AppV2/Utils/trade-params-utils';\nimport { useTraderStore } from 'Stores/useTraderStores';\n\ntype TNewValues = {\n amount?: string | number;\n payout_per_point?: string | number;\n barrier_1?: string | number;\n has_take_profit?: boolean;\n has_stop_loss?: boolean;\n take_profit?: string | number;\n stop_loss?: string | number;\n};\n\nexport const usePropo..."
},
{
"name": "packages/trader/src/Stores/Modules/Trading/Helpers/proposal.ts",
"totalCharacters": 188,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": "import { TPriceProposalResponse, TSocketError } from '@deriv/api';\nimport { TTradeStore } from 'Types';\n response: TPriceProposalResponse & { error?: TSocketError<'proposal'>['error'] }"
},
{
"name": "packages/trader/src/Stores/Modules/Trading/trade-store.ts",
"totalCharacters": 61,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": " [contract_type]: getProposalInfo(this, response),"
},
{
"name": "packages/trader/src/trader-providers.tsx",
"totalCharacters": 164,
"aiCharacters": 0,
"percentage": 0,
"status": "modified",
"hasAddedContent": true,
"addedContent": "import { APIProvider } from '@deriv/api';\n <APIProvider>\n <TraderStoreProvider>{children}</TraderStoreProvider>\n </APIProvider>"
}
]
}🚀 Analysis powered by ShiftAI • Analyzes only PR changes, not entire files |
|
Claude finished @nijil-deriv's task —— View job Code Review Complete ✅I've completed a thorough review of PR #384. This PR refactors the custom 📊 Review SummaryHigh Priority Issues
Medium Priority Issues
Low Priority Issues
🔍 Detailed AnalysisArchitecture & Design✅ Positive Changes:
TypeScript & Type SafetyIssues Found:
Recommendations:
Performance ConsiderationsPotential Issues:
Recommendations:
Security✅ No major security concerns identified
Minor note: Error objects from API may contain sensitive details - ensure they're sanitized before displaying to users TestingCoverage Analysis:
Gaps:
💡 Specific RecommendationsImmediate Fixes (Pre-merge):
const proposal_request = React.useMemo(() => {
const request = getProposalRequestObject({
new_values: proposal_request_values,
trade_store,
trade_type: contract_type,
});
// Validation logic here
if (should_skip_validation === 'take_profit' && request.limit_order?.take_profit) {
delete request.limit_order.take_profit;
}
if (should_skip_validation === 'stop_loss' && request.limit_order?.stop_loss) {
delete request.limit_order.stop_loss;
}
return request;
}, [proposal_request_values, trade_store, contract_type, should_skip_validation]);
// In components using useProposal
React.useEffect(() => {
return () => {
// Cleanup function to cancel any pending requests
invalidate('proposal');
};
}, []);
const isValidProposalRequest = (obj: unknown): obj is TPriceProposalRequest => {
// Add validation logic
return typeof obj === 'object' && obj !== null && 'proposal' in obj;
};
// Then use it:
if (!isValidProposalRequest(proposal_request)) {
throw new Error('Invalid proposal request');
}Follow-up Tasks (Post-merge):
🎯 Priority Breakdown
✅ Final VerdictOverall Assessment: APPROVE with recommendations This is a solid refactoring that improves code maintainability by standardizing on Key Strengths:
Suggested Actions Before Merge:
Post-Merge Actions:
The refactoring achieves its goal effectively. The identified issues are mostly about defensive programming and performance optimization rather than functional correctness. CI Status: ✅ 2 passed, 2 in progress |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Manifest Files |
nijil-deriv
changed the title
Deploying derivatives-trader with
|
| Latest commit: |
089a49a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2b2ab438.derivatives-trader.pages.dev |
| Branch Preview URL: | https://refactor-remove-usedtraderqu.derivatives-trader.pages.dev |
|
Claude finished @nijil-deriv's task —— View job Code Review Complete ✅I've completed a thorough review of PR #384. This PR successfully refactors the custom 📊 Review SummaryOverall Assessment: Approve with minor recommendations This is a well-executed refactoring that improves code maintainability by standardizing on Key Statistics
🎯 Priority IssuesHigh Priority1. Missing Memoization in
|
| Priority | Count | Blocking? |
|---|---|---|
| High | 3 | Consider fixing #1 |
| Medium | 3 | No, track in follow-up |
| Low | 2 | No |
✅ Final Verdict
Recommendation: APPROVE ✅
This refactoring successfully achieves its goals and improves the codebase:
Strengths
- Clean removal of custom query logic
- Proper migration to standard React Query patterns
- Comprehensive test coverage
- No breaking changes to component APIs
- Good performance optimizations with caching
Minor Concerns
- Missing memoization in one hook (easy fix)
- One type assertion that could be safer
- Some edge cases not covered in tests
The identified issues are mostly about defensive programming and optimization rather than functional correctness. The code works well as-is, but the recommendations would make it more robust.
Suggested Actions
- Optional pre-merge: Add memoization to
useProposalhook - Post-merge: Monitor for any performance regressions
- Follow-up: Add missing test cases and improve error handling
Great work on this refactoring! The migration from custom hooks to standard React Query patterns is well-executed. 🎉
CI Status: ✅ 2 passed, 2 in progress | View CI
| Branch: refactor/remove-useDtraderQuery
|
Claude finished @nijil-deriv's task —— View job Code Review Complete ✅I've completed a thorough review of PR #384, focusing on correctness, code quality, performance, security, and testing. 📊 Overall AssessmentVerdict: Approve with Minor Recommendations ✅ This is a well-executed refactoring that successfully migrates from custom Key Metrics:
🎯 Priority FindingsHigh Priority1. Missing Memoization in
|
| Priority | Count | Blocking? | Should Address |
|---|---|---|---|
| High | 2 | No | Consider #1 for performance |
| Medium | 4 | No | Track in follow-up |
| Low | 2 | No | Optional improvements |
🎉 Conclusion
This refactoring successfully achieves its goals and improves the codebase. The migration from custom hooks to standard React Query patterns is well-executed with comprehensive test updates and no breaking changes.
Key Achievements:
- Reduced custom code by ~200 lines
- Standardized data fetching patterns
- Improved type safety (mostly)
- Maintained functional correctness
- Good performance characteristics with caching
Minor Issues:
The identified issues are mostly about defensive programming and optimization rather than functional correctness. The code works well as-is, but the recommendations would make it more robust and performant.
Suggested Actions:
- Recommended: Add memoization to
useProposalhook before merge - Optional: Address type assertion for better type safety
- Post-merge: Monitor performance and track follow-up improvements
Great work on this refactoring! The codebase is cleaner and more maintainable. 🚀
CI Status: ✅ 1 passed, 3 in progress | View CI
| Branch: refactor/remove-useDtraderQuery
|
Claude finished @nijil-deriv's task —— View job Code Review Complete ✅I've completed a comprehensive review of PR #384, which refactors the custom 📊 Overall AssessmentVerdict: ✅ APPROVE with Minor Recommendations This is a well-executed refactoring that successfully standardizes data fetching patterns by migrating from custom hooks to Key Metrics:
🎯 Findings by Priority✅ StrengthsArchitecture & Design
Code Quality
Performance
Security
|
2 participants
🤖 Auto-generated PR
This PR was automatically created by ShiftAI CLI.
chore: remove unclosed AI tag