Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Introducing TypeScript best practices #403
Introducing TypeScript best practices #403
Changes from 9 commits
f65e662
4056895
97d1c5b
33799f8
6d5ab94
24e3111
47a9b99
655c872
70ef156
8b59eec
23fde2d
b7eaafd
7845b74
9075ea3
986e34b
019b414
992e83c
3563bde
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Ideally this would be avoided if possible - casting as
any
should nearly always fix this issue. If you're having to use this - it may be a case where the engineer could ask for someone more experienced with TS to assist with the issue. Would also work as a great learning experience.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.
In my experience the easiest way to assist with TS types for other engineers is once it's in a PR and I can see all the code. I've found Slack messages in isolation without seeing all the code hard to answer. So perhaps encourage them to get their work done with
any
and then TS types support comes once the PR is open. It also leaves a feedback trail for any other engineers on the projectThere 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.
@kasparszarinovs I agree but I also feel like we need this as a last resort. Like @tobeycodes said this could potentially be a easy fix during code review.
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.
While this is true, I'd say it's been a while since React 18 is out (2 years!) While it doesn't harm I don't feel this is as relevant now? Just flagging to avoid this feeling dated when it's born but feel free to disregard
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.
The only reason I added this was bc there might be projects still on React 17.