Skip to content
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

Implement TypeScript in New Expensify #16665

Closed
19 of 20 tasks
roryabraham opened this issue Mar 29, 2023 · 192 comments
Closed
19 of 20 tasks

Implement TypeScript in New Expensify #16665

roryabraham opened this issue Mar 29, 2023 · 192 comments
Assignees
Labels
Engineering NewFeature Something to build that is a new item. Weekly KSv2

Comments

@roryabraham
Copy link
Contributor

roryabraham commented Mar 29, 2023

Proposal: Utilize TypeScript to reduce bugs in NewDot

πŸ“„ Link to the design doc

docs.google.com/document/d/1LNcgYeYyovQ88gNjLIgBqBReLtbLAbxeAbna9QKxeAQ/edit#

πŸ–‡οΈ Other links

Strategy

Our strategic roadmap for the next couple years is Reunification – moving all of our existing users from OldDot to NewDot, and it’s going to look like a completely new product. During this transition period, there’s an increased risk for churn, and stability of the platform is more important than ever. Any lines of defense between our customers and bad code will pay dividends during any tumultuous times to come.

Problem

JavaScript has been very popular for front-end development for a long time, and over the years it has evolved with new developer-friendly features that make it easier to GSD. Overall, it’s pretty great. However, it is still plagued by one fundamental problem: runtime errors stemming from incorrect types. This common class of problem leads to bugs, ranging in severity from showing weird things on the screen to crashing the whole app.

While there are many bugs that can be caused by incorrect types, the most common example of a runtime type safety error is Cannot access property X of undefined. A quick GitHub search reveals that over the last couple years we have introduced hundreds of this type of bug into the NewDot codebase, each of them stemming from this same root cause – attempting to access a property on a variable whose value is undefined. When this occurs, the app crashes.

Solution

Let's commit to adopting TypeScript in New Expensify, an awesome superset of JavaScript that gives us the benefits of using a compiled language like C++! That way, type errors can be caught at compile-time instead of run-time, and a whole class of common bugs will be eliminated from our product. #BugZero here we come.

Furthermore, let’s engage the near-limitless capacity of our open-source community to do it in parallel to our existing roadmap, without pausing any other feature development in the meantime.

Tasks

  • Post Proposal (full Problem/Solution statement) in #expensify-open-source
  • Wait at least one full business day, and until the post has a majority (2/3) of positive reactions (πŸ‘)
  • Paste Proposal in the space above with a link to the Slack thread
  • Email [email protected] and paste in the Proposal
  • Fill out the High-level overview of the problem, Timeline, and Terminology sections of the Design Doc
  • Email [email protected] (continue the same email chain as before) with the link to your Design Doc
  • Host a pre-design meeting (example) in #expensify-open-source to discuss any necessary details in public before filling out the High-level of proposed solution section.
  • Fill out the High-level of proposed solution section
  • Email [email protected] again with links to the doc and pre-design conversation in Slack
  • Add the DesignDocReview label to get the High-level of proposed solution section reviewed
  • Respond to any questions or concerns and bring up blockers in Slack to get a consensus if necessary
  • Confirm that the doc has the minimum necessary number of reviews before proceeding
  • Host another pre-design meeting in #expensify-open-source to ask for engineering feedback on the technical solution.
  • Fill out the Detailed implementation of the solution and related sections.
  • Re-add the DesignDocReview label to this issue
  • Respond to any questions or concerns and bring up blockers in Slack to get consensus if necessary
  • Confirm that the doc has the minimum necessary number of reviews before proceeding
  • Email [email protected] one last time to let them know the Design Doc is moving into the implementation phase
  • Implement the changes
  • Send out a follow up email to [email protected] once everything has been implemented and do a Project Wrap-Up retrospective that provides:
    • Summary of what we accomplished with this project
    • What went well?
    • What could we have done better?
    • What did we learn?
@roryabraham roryabraham added Weekly KSv2 NewFeature Something to build that is a new item. labels Mar 29, 2023
@roryabraham roryabraham self-assigned this Mar 29, 2023
@MelvinBot

This comment was marked as resolved.

@roryabraham
Copy link
Contributor Author

Here's a high-level overview of what I'm imagining for the rollout plan:

flowchart LR
    A(Setup TypeScript for E/App source code)
    X(Setup TypeScript for GitHub Actions)
    Y(Setup TypeScript for Jest)

    B(Laying Foundation)
    C(Materials and Plumbing)
    D(Construction)
    E(Finishings)
    
    F(Implement TypeScript in GitHub Actions)
    G(Implement TypeScript for Jest tests)
    
    subgraph Main App
        A --> B --> C --> D --> E
    end
    
    subgraph GitHub Actions
        direction LR
        X --> F
    end
    
    subgraph Jest
        Y ----> G
        C --> G
    end
    
    H(Setup TypeScript in react-native-onyx)
    I(Implement TypeScript in react-native-onyx)
    
    subgraph react-native-onyx
        direction LR
        H --> I
    end
Loading

Most of this is pretty self-explanatory, but I'll go into some extra detail for the rollout plan for the main source code.

  • Setup:
    • Add TypeScript tooling for local development and in the deploy process.
    • Migrate App.js to App.ts
  • Laying Foundation:
    • Create type definitions for key entities (such as Report and Policy)
    • Migrate simple, widely used files that will use mostly inferred types (such as CONST and ONYXKEYS)
    • After this phase, disallow new JS files
  • Materials and Plumbing:
    • Migrate low-level components and utilities that don’t depend much on other files. Because these files generally don’t have many dependencies, it’s less likely that we’ll scope creep, have tons of conflicts, or find that they need to be on HOLD for other migrations.
    • Files that don’t have other dependencies are like β€œmaterials”. Starting with most of the basic materials in place will make the rest of the construction go faster and smoother.
    • However, we won’t delay the main β€œconstruction phase” to get all the materials in place, just the most common and critical ones.
  • Construction:
    • Migrate our most commonly used and most critical components and libraries.
  • Finishings:
    • Finish migrating, until there are no JS files in our App source code

We can use deprank to determine the number of dependencies and dependents on a file-by-file basis.

@melvin-bot melvin-bot bot added the Overdue label Apr 10, 2023
@roryabraham
Copy link
Contributor Author

Posted a second predesign here yesterday

@melvin-bot melvin-bot bot removed the Overdue label Apr 19, 2023
@melvin-bot melvin-bot bot added the Overdue label May 1, 2023
@roryabraham
Copy link
Contributor Author

@melvin-bot melvin-bot bot removed the Overdue label May 2, 2023
@melvin-bot melvin-bot bot added the Overdue label May 11, 2023
@neil-marcellini
Copy link
Contributor

neil-marcellini commented May 17, 2023

I'm going to help Rory with this by asking external agencies to collaborate on a pre-design / writing the doc, and then making sure the process is going well.

@neil-marcellini neil-marcellini self-assigned this May 17, 2023
@neil-marcellini neil-marcellini added Daily KSv2 and removed Weekly KSv2 labels May 17, 2023
@melvin-bot melvin-bot bot removed the Overdue label May 17, 2023
@neil-marcellini
Copy link
Contributor

I didn't get to this today.

@neil-marcellini
Copy link
Contributor

I posted to the Callstack and Software Mansion Slack channels asking for proposals on how they would plan and implement this project.

https://expensify.slack.com/archives/C03UK30EA1Z/p1684525440666989
https://expensify.slack.com/archives/C04878MDF34/p1684525456740769

@melvin-bot melvin-bot bot added the Overdue label May 22, 2023
@melvin-bot
Copy link

melvin-bot bot commented May 22, 2023

@neil-marcellini, @roryabraham Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@neil-marcellini
Copy link
Contributor

I got a reply from Callstack and SWM wants a day or two to post something.

@neil-marcellini
Copy link
Contributor

I created #typescript-new-expensify and invited CK and SWM. @hayata-suenaga is helping out now too!

I still need to read through the proposal that software mansion already wrote, and then I’ll let them know that we’re waiting for their joint proposal and ask on an ETA for that. Maybe Hayata could tackle that?

@hayata-suenaga
Copy link
Contributor

@neil-marcellini thank you for adding me to this issue

I thought you asked for proposals separately from Software Mansion and CallStack. Do they know they have to produce a joint proposal?

@hayata-suenaga
Copy link
Contributor

okay CallStack is willing to collaborate

  1. Proposal: We will review the proposal published in March by Fabio and other proposals and comments from contributors to gather the most relevant information and update the proposal and plan in cooperation with SWM

from Tomek

@hayata-suenaga
Copy link
Contributor

and Software Mansion is saying they're already in touch with CallStack

This is our proposal on how to tackle migration, and we are already in touch with Callstack to compile a common one

From Michal

@melvin-bot melvin-bot bot added the Overdue label Jul 16, 2024
@roryabraham
Copy link
Contributor Author

I went ahead and emailed out the project wrap-up. The last things couple items are still lingering, but 99.9% of this project is done

@melvin-bot melvin-bot bot removed the Overdue label Jul 17, 2024
@melvin-bot melvin-bot bot added the Overdue label Jul 25, 2024
@roryabraham
Copy link
Contributor Author

One of the last two wrap-up issues was rejected by maintainer so we closed the issue as not planned. That leaves only #43055

@melvin-bot melvin-bot bot removed the Overdue label Jul 27, 2024
@fabioh8010
Copy link
Contributor

Another Post-migration weekly update on the Slack channel!

@neil-marcellini
Copy link
Contributor

We're incredibly close to finishing this for good, not overdue!

@neil-marcellini
Copy link
Contributor

Head's up, I'll be working 50% for the rest of this week and next 🌲

@roryabraham
Copy link
Contributor Author

@ShridharGoel reports that the last PR to close this out will be ready for review in the next couple days

@fabioh8010
Copy link
Contributor

Another Post-migration weekly update on the Slack channel!

@melvin-bot melvin-bot bot added the Overdue label Aug 23, 2024
@roryabraham
Copy link
Contributor Author

Just merged Expensify/eslint-config-expensify#114 earlier today

@melvin-bot melvin-bot bot removed the Overdue label Aug 23, 2024
@melvin-bot melvin-bot bot added the Overdue label Sep 2, 2024
@roryabraham
Copy link
Contributor Author

Apparently some delays for personal reasons on the part of contributor for the last PR.

hopefully we can close this out by EOW 🀞🏼

@melvin-bot melvin-bot bot removed the Overdue label Sep 3, 2024
@melvin-bot melvin-bot bot added the Overdue label Sep 11, 2024
@neil-marcellini
Copy link
Contributor

Still waiting on that last one.

@melvin-bot melvin-bot bot removed the Overdue label Sep 12, 2024
@fabioh8010
Copy link
Contributor

Another Post-migration weekly update on the Slack channel!

@melvin-bot melvin-bot bot added the Overdue label Sep 20, 2024
@neil-marcellini
Copy link
Contributor

Here's the latest update.

@roryabraham it seems like it's going to take quite a while before withOnyx is deprecated. Can we sent the project wrap up sooner?

@melvin-bot melvin-bot bot removed the Overdue label Sep 23, 2024
@roryabraham
Copy link
Contributor Author

I already sent the project-wrap up for this project. The only thing left in this project is #43055

@roryabraham
Copy link
Contributor Author

I can track the official close of the project in https://github.com/Expensify/Expensify/issues/413336. We can close this issue. Thanks so much everyone for all your hard work and consistent communication. This was a huge project, and it's already hard to remember what it was like before we used TypeScript.

Huge thanks in particular to @fabioh8010 and @blazejkustra, who were the real champions of this project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering NewFeature Something to build that is a new item. Weekly KSv2
Projects
None yet
Development

No branches or pull requests