Fix: Add symbol format conversion for A-Share/HK stocks in TradingVie…#57
Conversation
|
Someone is attempting to deploy a commit to the ravixalgorithm's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds a formatter Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@lib/utils.ts`:
- Around line 157-158: The guard in formatSymbolForTradingView currently returns
the falsy input (which can be null/undefined) even though the function is
declared to return a string; change the guard to return an explicit empty string
('') when symbol is falsy so the function always returns a string; update the
early-return in formatSymbolForTradingView to return '' instead of symbol and
ensure any downstream logic expecting a string continues to work.
🧹 Nitpick comments (1)
lib/utils.ts (1)
162-174:String.replaceis not anchored to the end — useslicefor suffix removal.
.replace('.SS', '')removes the first occurrence, not necessarily the suffix. After anendsWithcheck,slice(0, -3)(or-3for.SS/.SZ/.HK) is more precise and avoids any edge-case mismatch.Proposed fix
// Shanghai if (upperSymbol.endsWith('.SS')) { - return `SSE:${upperSymbol.replace('.SS', '')}`; + return `SSE:${upperSymbol.slice(0, -3)}`; } // Shenzhen if (upperSymbol.endsWith('.SZ')) { - return `SZSE:${upperSymbol.replace('.SZ', '')}`; + return `SZSE:${upperSymbol.slice(0, -3)}`; } // Hong Kong if (upperSymbol.endsWith('.HK')) { - return `HKEX:${upperSymbol.replace('.HK', '')}`; + return `HKEX:${upperSymbol.slice(0, -3)}`; }
…View (CodeRabbit feedback)
There was a problem hiding this comment.
Pull request overview
Fixes TradingView widget “Invalid Symbol” issues for non‑US equities by converting Finnhub market suffix symbols (e.g., 600519.SS, 000001.SZ, 0700.HK) into TradingView’s expected exchange‑prefixed format (e.g., SSE:600519, SZSE:000001, HKEX:0700).
Changes:
- Added
formatSymbolForTradingView()utility to convert.SS/.SZ/.HKsymbols into TradingView exchange-prefixed symbols. - Updated the watchlist TradingView embed to use the formatted symbol for the widget input.
- Updated the stock detail page TradingView widgets to use the formatted symbol across all widget configs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
lib/utils.ts |
Adds a symbol conversion helper for TradingView-compatible symbols. |
components/watchlist/TradingViewWatchlist.tsx |
Applies the formatted symbol to the TradingView market-quotes widget input. |
app/(root)/stocks/[symbol]/page.tsx |
Uses the formatted symbol for all embedded TradingView widgets on the stock detail page. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import React, { useEffect, useRef, memo } from 'react'; | ||
| import { useTheme } from "next-themes"; | ||
| import { formatSymbolForTradingView } from '@/lib/utils'; |
There was a problem hiding this comment.
useTheme is imported but never used in this component. This will fail ESLint/TypeScript no-unused-vars checks; remove the import or actually use it to set the TradingView widget theme dynamically.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| const symbolList = symbols.map(s => ({ | ||
| name: s, | ||
| name: formatSymbolForTradingView(s), | ||
| displayName: s | ||
| })); |
There was a problem hiding this comment.
This watchlist widget still uses the unformatted raw symbol as displayName while only name is formatted. If the intent is that watchlist entries “show and use” the formatted TradingView symbols (per PR description), displayName should also use formatSymbolForTradingView(s) (or the PR description should be updated).
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| return `SSE:${upperSymbol.replace('.SS', '')}`; | ||
| } | ||
|
|
||
| // Shenzhen | ||
| if (upperSymbol.endsWith('.SZ')) { | ||
| return `SZSE:${upperSymbol.replace('.SZ', '')}`; | ||
| } | ||
|
|
||
| // Hong Kong | ||
| if (upperSymbol.endsWith('.HK')) { | ||
| return `HKEX:${upperSymbol.replace('.HK', '')}`; |
There was a problem hiding this comment.
upperSymbol.replace('.SS', '') (and the similar .SZ/.HK cases) removes the first occurrence, not necessarily the suffix you just checked for. Prefer stripping only the trailing market suffix (e.g., with a suffix slice or a /\.SS$/-style replacement) so symbols containing the substring earlier don’t produce incorrect output.
| return `SSE:${upperSymbol.replace('.SS', '')}`; | |
| } | |
| // Shenzhen | |
| if (upperSymbol.endsWith('.SZ')) { | |
| return `SZSE:${upperSymbol.replace('.SZ', '')}`; | |
| } | |
| // Hong Kong | |
| if (upperSymbol.endsWith('.HK')) { | |
| return `HKEX:${upperSymbol.replace('.HK', '')}`; | |
| return `SSE:${upperSymbol.slice(0, -3)}`; | |
| } | |
| // Shenzhen | |
| if (upperSymbol.endsWith('.SZ')) { | |
| return `SZSE:${upperSymbol.slice(0, -3)}`; | |
| } | |
| // Hong Kong | |
| if (upperSymbol.endsWith('.HK')) { | |
| return `HKEX:${upperSymbol.slice(0, -3)}`; |
|
@CrixusWen Thanks a lot for fixing and opening the PR, really appreciated |
…-symbol-format
Title: Fix TradingView widget invalid symbol issue for non-US markets
Description: Finnhub returns symbols like 600519.SS (Shanghai) or 000001.SZ (Shenzhen), but TradingView widgets expect SSE:600519 or SZSE:000001.
This PR adds a utility function
formatSymbolForTradingView
to handle this conversion automatically, fixing the "Invalid Symbol" error for A-share and HK stocks.
Summary by CodeRabbit