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
Migrate from create-react-app-typescript to official CRA #191
Conversation
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.
Looks good. I left some suggestions that could be done later.
@@ -15,7 +15,7 @@ import ProfessorPeopleView from './pages/ProfessorPeopleView'; | |||
import { Analytics } from './includes/Analytics'; | |||
import { Loader } from 'semantic-ui-react'; | |||
import { userUpload } from '../firebasefunctions'; | |||
import { useMyCourseUser } from 'src/firehooks'; | |||
import { useMyCourseUser } from '../firehooks'; |
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.
Latest CRA can actually support it. You can set the baseUrl
in tsconfig. Not push blocking.
export class Analytics extends React.Component { | ||
props: AnalyticsProps; | ||
props!: AnalyticsProps; |
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
export class Analytics extends React.Component<AnalyticsProps> {}
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.
Makes sense. I just did this to get it to compile. If I have time, I'd love to make it a hooks component
* Refactor ProfOHInfo * Progress * More Progress * Remove unused packages * Remove unused imports * Fix comparisons * I wish moment had types * Fix broken TA dropdown * Migrate from create-react-app-typescript to official CRA (#191) * Migrate to create-react-app * Fix crashes * var -> let * Check in safe API key This key is designed to be public Co-authored-by: Ryan Slama <Glitched@users.noreply.github.com>
Replace create-react-app-typescript with the official create-react-app. Create-react-app-typescript is deprecated and no longer receiving updates in favor of official typescript support.
Inside This PR
Notes
Moving away is essential for the long term health of the project and I figured I would do it was I was here because I've spent more time fighting with Typescript than most and some of the changes were non-obvious.
I fixed all the breaking errors, but this created a bunch of warnings (where we were doing bad stuff). We should probably get around to fixing all of them at some point, but I think an incremental adoption strategy would work best.
Future work involves moving from the deprecated TSLint to ESLint. Then we should improve our linting for accessibility, code format, and code safety.
Breaking Changes
/src/components/types/
Checklist: