-
Notifications
You must be signed in to change notification settings - Fork 2
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
Refactoring and Improving Price related API Route #290
Conversation
Run & review this pull request in StackBlitz Codeflow. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
src/components/client/StatsPage.tsx
Outdated
@@ -164,7 +168,7 @@ const StatsPage = () => { | |||
> | |||
{isFailedToFetchData |
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.
Maybe we rename showData to a component that accepts <StatInfo ...{props} /> then we use composition api to render the outcome.
src/utils/cache.ts
Outdated
res: NextApiResponse, | ||
cacheHeader = `public, s-maxage=${config.CACHE_DURATION_SECONDS}, stale-while-revalidate=${config.CACHE_STALE_WHILE_REVALIDATE_SECONDS}`, | ||
) => { | ||
res.setHeader('Cache-Control', cacheHeader) |
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.
res.setHeader('Cache-Control', cacheHeader) | |
import config from '@/config' | |
import { NextApiResponse } from 'next' | |
export const setCacheHeaders = ( | |
res: NextApiResponse, | |
cacheHeader = `public, s-maxage=${config.CACHE_DURATION_SECONDS}, stale-while-revalidate=${config.CACHE_STALE_WHILE_REVALIDATE_SECONDS}` | |
): void => { | |
const headers = [ | |
['Cache-Control', cacheHeader], | |
['CDN-Cache-Control', cacheHeader], | |
['Vercel-CDN-Cache-Control', cacheHeader] | |
] as const | |
headers.forEach(([name, value]) => res.setHeader(name, value)) | |
} | |
src/components/client/StatsPage.tsx
Outdated
<span>Loading...</span> | ||
</Spinner> | ||
) | ||
const ShowData = ({ |
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 rename this to
const NumericDisplay = ({
value,
prefix,
isFetched,
isFailedToFetchData
}: NumericDisplayProps) => {
// ... same implementation
}
Also, we could handle the isFailedToFetchData inside it not outside, i.e
interface NumericDisplayProps {
value: number | undefined
prefix?: string
isFetched?: boolean
isFailedToFetchData?: boolean
}
const NumericDisplay = ({
value,
prefix,
isFetched,
isFailedToFetchData
}: ShowDataProps) => {
if (isFailedToFetchData) {
return '-'
}
if (value === undefined && !isFetched) {
return (
<Spinner variant="primary" role="status" size="sm">
<span>Loading...</span>
</Spinner>
)
}
return value ? (prefix || '') + Humanize.formatNumber(value) : '-'
}
Code Climate has analyzed commit 6f7383d and detected 0 issues on this pull request. View more on Code Climate. |
useRadio(radioProps) | ||
return ( | ||
<chakra.label | ||
{...htmlProps} | ||
cursor={state.isDisabled ? 'not-allowed' : 'pointer'} | ||
> | ||
<input {...getInputProps({})} hidden /> | ||
<Box {...getCheckboxProps()}> | ||
<Box {...getRadioProps()}> |
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.
Missed this, why radio props?
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.
getCheckboxProps is deprecated
Refactoring and Improving Price-related API Route
This PR works on improving the caching on price related API route to reduce load on server, Resolve issue with twitter following API continuous loading.
Linked issues
closes EveripediaNetwork/issues#3728