-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
refactor(website): convert website to TypeScript #5328
Conversation
✔️ [V2] 🔨 Explore the source changes: c211bef 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/61126c00390ac10007324620 😎 Browse the preview: https://deploy-preview-5328--docusaurus-2.netlify.app |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-5328--docusaurus-2.netlify.app/ |
✔️ [V2] 🔨 Explore the source changes: 801c694 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/61138a849b5dc70007919153 😎 Browse the preview: https://deploy-preview-5328--docusaurus-2.netlify.app |
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
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.
Thanks for working on this!
That looks good overall, would just replace some types by React.ComponentProps
:)
@@ -60,7 +66,7 @@ const COLOR_SHADES = { | |||
|
|||
const DEFAULT_PRIMARY_COLOR = '3578e5'; | |||
|
|||
function ColorGenerator({children, minHeight, url}) { | |||
function ColorGenerator() { |
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.
oh, this is weird that nothing catched those unused 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.
This is because we have set 'no-unused-vars': OFF,
in ESLint config 😅 When converted to TS this is caught by typescript compiler itself
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.
oh I see, @typescript-eslint/no-unused-vars
would catch it
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.
Unfortunately still not for the website 🤦♂️
Line 9 in 9afc900
website/ |
I was wondering why we had this. Should we re-enable ESLint in the website?
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.
probably a mistake
we should likely have a dedicated eslint config for our own website (maybe using the one of the FB template or similarr?)
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
Signed-off-by: Josh-Cena <sidachen2003@gmail.com>
LGTM thanks 👍 good catch for the edit URL 😅 |
Motivation
Originally from #5233 (comment). Dogfood TS support on the official Docusaurus website, while also enforcing type checks for other contributors.
Have you read the Contributing Guidelines on pull requests?
Yep
Test Plan
Build passes; pages like:
are working as intended