-
Notifications
You must be signed in to change notification settings - Fork 75
The Great AntAlmanac TypeScript Rewritening™
https://github.com/icssc/AntAlmanac/pull/411
This is a dev log about rewriting our project in TypeScript. So far the only author is @EricPedley.
- First thing I did was follow the Create-React-App tutorial for installing TypeScript on our project: https://create-react-app.dev/docs/adding-typescript/
Why TypeScript? - It makes the developer experience easier. With TypeScript, we get to know exactly what to pass as parameters to a function or what fields an object has with IDE autocomplete, and you get compiler errors for messing anything up instead of it silently failing until it shows up as a runtime error, which is a big problem for us because we don't have unit tests to automatically catch those.
-
Ran the local dev server to see if everything compiled. It did locally but not on our staging environment because of a peer dependency conflict between the TypeScript version in our package and the one that
react-scripts
depends on, so I updatedreact-scripts
and luckily it fixed it. -
Renamed a single file to
.tsx
and fixed the resulting compile errors. Making changes one at a time like this will temporarily mess up the git history, but once we squash and merge it will be reduced to one commit. We can add that commit to our.git-blame-ignore-revs
, which tellsgit blame
what commits to ignore, so we can ignore that one commit.
the difference between the
.ts
and.tsx
files is the presence of the embedded HTML-in-JS snippets like and such, which are called Components in React.
- Realized the huge scope of this task and split it up into 4 smaller tasks to complete, each for rewriting a different section of the codebase. This will still end up fine because we can merge them into the parent PR and then squash&merge that one, making the whole rewrite only 1 commit that we can ignore on git blame.
Getting involved with more complicated details.
- Just had a tough time rewriting the ConditionalWrapper component in TS, hoping to use this example to illustrate how to read TS error messages. Read it and then read on. The error message looks intimidating but when you break it down is actually makes a lot of sense when we break it down.
The | (pipe) character between types, like
Element | ReactElement
is a type union, meaning that either Element or ReactElement are valid. In a parameter type annotation, it means you can pass either as args and it will work. In a return type annotation, it means that you can safely assume the function will return one or the other of those types.
-
First line: The part I underlined in red is just the type structure of the arrow function we're passing as an argument to forwardRef. The part underlined in green is the type that forwardRef expects as its parameter.
-
Second line: This is explaining why the red and green types are incompatible. Their return types are different. The blue box and underline are the return type of the function we passed, and the yellow underline is the expected return type of the function we pass to forwardRef.
-
Third line: This explains why the function return types differ. The blue one has
Element
in the union, while the second one doesn't. This means that our function might return an Element, which is somehow incompatible with ReactElement. If we hover overwrapper(children)
, it tells us exactly why they're incompatible (underlined in pink): ReactElement has some properties Element doesn't such asprops
. So, if forwardRef's code calls our function and tries to access theprops
value of the returned object, it might be undefined, so it doesn't let us possible pass a value like that. So, the fix? Change the type of children to ReactElement to guarantee it has all the right properties.
-
This is harder than I thought it would be. I'm having trouble rewriting the News component because its state has two properties that are initialized to null, and later in functions it has checks for if those properties are non-falsy and their length is not zero, but the problem is that since they're intiialized to undefined, the compiler thinks that the second part that checks the length is unreachable, so it assigns the length property the 'never' type since it thinks that it won't be able to get it because that state value is undefined, even though it is updated in another function to be not undefined. Using null gives the same problem. Might have to resort to posting on StackOverflow for this one because I'm not finding existing answers, or rewrite the component as a functional component instead of a class component.
-
I just type-annotated state directly on the line it was initialized and it fixed the problem. I was already passing in an interface for state as a parameter to PureComponent but I guess the type system didn't infer that
state
was of that type, so I just needed to explicitly say it. Another problem I spent a lot of time on was where ConditionalWrapper wasn't letting me pass an array of elements into it as a child, and I was confused because the specific array I was passing also contained ConditionalWrappers so I didn't see that the error was coming from the parent conditional wrapper. Once I read that, the error made sense that I was passing an array of elements, not a single one, so it couldn't cast right. I fixed it by just wrapping the list in a fragment.
Before:
After:
And all the references to state worked without any further code changes (except renaming eventName
to title
for consistency) 😎
Can you tell what's wrong with this code? It's not immediately clear anything is wrong, until we add TYPESCRIPT
The string for the action is being passed to the label prop instead of the action prop like it should be. With TS, it's an easy catch. I wonder who wrote this buggy code in the first place ... 😳
This one belongs on The Daily WTF
So, our site has a button to screenshot your schedule from the calendar view.
That component is nested like so:
- ScheduleCalendar (red box)
- CalendarPaneToolbar (yellow box)
- ScreenshotButton (green box)
- CalendarPaneToolbar (yellow box)