App: [HOLD for payment 2024-01-05] [$500] Prohibit the use of defaultProps in ts functional components
Problem
Our TypeScript guidelines state that functional components should use prop destructuring instead of defaultProps, but it’s still possible for developers who don’t know that to try and use defaultProps. In the worst case, that means we’ll have inconsistent patterns for default props in our TS functional components. In a more typical case, the development cycle is slowed because we have to request changes during review and wait for the developer to make those changes.
Solution
Set up an ESLint rule that prohibits the use of defaultProps in TypeScript functional components, and directs developers to use prop destructuring instead. To be clear, this means that defaultProps should be allowed:
- in JavaScript files
- in TS files that contain class components (there should not be many of these, but some may exist)
- it’s not ideal if we have to suppress the linter to make this case work, but it’s an option.
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~01e2d42d1ab75ab97a
- Upwork Job ID: 1735845538188914688
- Last Price Increase: 2023-12-20
- Automatic offers:
- alitoshmatov | Reviewer | 28063449
- ishpaul777 | Contributor | 28063450
About this issue
- Original URL
- State: closed
- Created 7 months ago
- Comments: 34 (18 by maintainers)
Proposal
Please re-state the problem that we are trying to solve in this issue.
Prohibit the use of defaultProps in ts functional components
What is the root cause of that problem?
N/a, not a bug more of dev experience improvement
What changes do you think we should make in order to solve the problem?
At present we suppressed the linter rule for default props 'react/require-default-props` for typescript files here, we just need to enable the rule and set option ‘functions’ to ‘defaultArguments’.
‘react/require-default-props’: [‘error’, {functions: ‘defaultArguments’}]`
Note: Enabling this would mean if there are optional props without defaultArguments linter will throw error, we need to provide “undefined” as default
@ishpaul777 $500 payment sent and contract ended - thank you! 🎉
@abekkala no need payment for me since I did not review the PR
No regression tests needed here
PR merged
eslint-config-expensify PR merged, back to E/App PR now
Adjusting the bounty back to $500
@strepanier03
STATUS
I’m going ooo through Jan 02. This is at the finish line and a PR has already been created. Payment can be processed after the PR is deployed to prod, and waiting period has passed. pending no regressions
@ishpaul777 Fix $250 Offer link @alitoshmatov PR Review $250 Offer link
Thanks for assigning, I am working on the PR now, will raise shortly