[CT-3468] - bb/package-count #115
Conversation
|
pages/index.tsx
Outdated
| const [searchInput, setSearchInput] = useState(''); | ||
| const router = useRouter(); | ||
|
|
||
| let packageCountFormat = packageCount==="all" ? " ": "font-bold" |
There was a problem hiding this comment.
We should remove this, we don't need it to be bold 👍
pages/index.tsx
Outdated
| //to prevent the case where site crashes due to error response | ||
| if (!response || response.status!==200) { | ||
| return { | ||
| props: { | ||
| packageCount: "all" | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I think we don't need this, if it errors, it will simply not have a packageCount
pages/index.tsx
Outdated
| Search all R packages on CRAN and Bioconductor | ||
| </h1> | ||
| <div className="text-xl md:text-2xl lg:text-3xl"> | ||
| Search <span className={packageCountFormat}>{packageCount}</span> R packages on CRAN and Bioconductor |
There was a problem hiding this comment.
packageCount should be number, that will be simpler for typing, in this case you want to add some logic to have all and then the number if it exists 🤝
pages/index.tsx
Outdated
|
|
||
| export default function HomePage() { | ||
| export default function HomePage({ packageCount }) { |
There was a problem hiding this comment.
It's nice to add the type of props, you can just do it inline here since it's only 1 prop
| export default function HomePage({ packageCount }) { | |
| export default function HomePage({ packageCount }: { packageCount?: number }) { |
pages/index.tsx
Outdated
| return { | ||
| props: { | ||
| packageCount: packageCount || '' | ||
| } | ||
| }; |
There was a problem hiding this comment.
You don't need a default value here since packageCount gets initialized either through the response or with null in the catch
There was a problem hiding this comment.
Also for info, if you set a default ``, then packageCount type becomes number | stringso better not do that 😅
pages/index.tsx
Outdated
| return { | ||
| notFound: true, | ||
| }; | ||
| packageCount = null; |
There was a problem hiding this comment.
Since you defined the type in the component to be { packageCount?: number } (it's an optional props), you can set a default value to undefined, this way the prop will be ignored
There was a problem hiding this comment.
Could you clarify this? When I set a packageCount to undefined it throws an JSON not serializable error
ben-brooker
force-pushed
the
bb/package-count
branch
from
661a694 to
9bd3ceb
Compare
ben-brooker
force-pushed
the
bb/package-count
branch
from
9bd3ceb to
8669239
Compare
| Search all R packages on CRAN and Bioconductor | ||
| </h1> | ||
| <div className="text-xl md:text-2xl lg:text-3xl"> | ||
| {`Search all ${packageCount > 0 ? `${packageCount.toLocaleString(undefined, {maximumFractionDigits:0})}`:''} R packages on CRAN and Bioconductor`} |
There was a problem hiding this comment.
It would be nice to turn this into a utility function that can be reused 👌
-After: