-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[HOLD for payment 2024-02-07] [$250] [TS migration] Migrate 'Form' component to TypeScript #31972
Comments
Hey! This is something that was recently implemented by @kowczarz, I think he should take this one 😄 |
@blazejkustra WDYT about doing InputWrapper and in a separate PR? We can suppress this error temporally, I think it should be enough. // src/components/Form/InputWrapper.tsx
// @ts-expect-error TODO: Remove this when FormContext is migrated to TS.
const {registerInput} = useContext(FormContext); This way we could unblock ~47 pages. Do you think is it possible or it makes sense? |
InputWrapper and what else? |
@blazejkustra I was thinking about this and Anyway, I tested the migration of this file yesterday and was able to do it, here is the branch. |
@fabioh8010 If this really unblocks almost 50 pages, I think we should do it! Especially looking at your branch which is a fairly simple migration 😄 |
FYI I started working on this issue this week, here is a draft PR |
This issue has not been updated in over 15 days. eroding to Monthly issue. P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do! |
Triggered auto assignment to @dangrous, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
Job added to Upwork: https://www.upwork.com/jobs/~01b00649428ca23975 |
This comment was marked as outdated.
This comment was marked as outdated.
i can work on this issue |
I can work on this issue, I applied to the Upwork job post |
📣 @LeOndaz! 📣
|
@RohanSasne @AnshuAgarwal24 @abzokhattab @LeOndaz This issue isn't external, so please do not comment for assignment 😄 (PR is almost merged) |
Contributor details |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.33-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-02-07. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
@allroundexperts, can you please complete the task above? Thanks! |
I don't think we need any new regression steps - if this PR does what it should, there should be no change at all (for users) from the existing Form! |
Thanks for clarifying, Daniel! |
Payment Summary
BugZero Checklist (@isabelastisser)
|
Reviewing. |
Payment Summary Reviewer: @allroundexperts owed $250 via NewDot PENDING payment in NewDot. |
Closing this, but we're still pending Payment in NewDot. cc @JmillsExpensify ^^ |
$250 approved for @allroundexperts based on summary above. |
TypeScript migration
Make sure you read through our TypeScript's style guide, cheatsheet and PropTypes conversion table before you start working on this migration issue.
Files
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: