-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ECO-2493] Market performance optimization #409
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Remove unused fields from swap/chat events when fetching from the indexer (based on what the market page uses, it's like 4-5 fields total, you can get rid of most of the market state/periodic state event stuff)
- Cache the swap/chat data like we do with candlesticks
- Consolidate the safe number parsing logic into a single function with a try/catch that returns false if it throws an error
|
||
export async function GET(request: NextRequest) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize after trying to help Greg with our /api routes, we should probably add a simple example as a jsdoc string above all of our GET routes/requests
You can probably write one up really quick with ChatGPT or something, should be pretty straightforward, just ask it to write a jsdoc but give it the context on the search params and any caveats, etc, then tell it to write an example
fromMarketNonce: string; | ||
}; | ||
|
||
const isNumber = (s: string) => !isNaN(parseInt(s)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const isNumber = (s: string) => !isNaN(parseInt(s)); | |
const isNumber = (s: string) => { | |
try { | |
return !isNaN(parseInt(s)); | |
} catch { | |
return false; | |
} | |
} |
Should always wrap these with a try/catch since it's coming from a search param and a malformed request somewhere will throw an error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll note that parseInt("lol")
just returns NaN
so I don't think this is a problem.
fromMarketNonce: string; | ||
}; | ||
|
||
const isNumber = (s: string) => !isNaN(parseInt(s)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not repeat this, use the other isNumber, and find the other one we use (I think I wrote a similar one somewhere, maybe like safeParseInt or something, it was for another route/fetcher/getter)
const marketID = Number(params.marketID); | ||
const fromMarketNonce = Number(params.fromMarketNonce); | ||
|
||
const res = await fetchSwapEvents({ marketID, fromMarketNonce }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably cache these like we do with candlestick data- although we might want to strip most of the data first (since I think we quite literally only use like 3-4 fields from this endpoint), but it's a lot better than hitting the indexer
It should also be simpler to cache these compared to candlesticks
const marketID = Number(params.marketID); | ||
const fromMarketNonce = Number(params.fromMarketNonce); | ||
|
||
const res = await fetchChatEvents({ marketID, fromMarketNonce }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should cache these as well (like the swap events)
Description
This PR optimizes the market page by:
Also switch base for quote on the chart volume.