Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Typescript refactor for olympus-frontend #239

Closed
0xdavinchee opened this issue Jul 19, 2021 · 3 comments
Closed

Typescript refactor for olympus-frontend #239

0xdavinchee opened this issue Jul 19, 2021 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@0xdavinchee
Copy link
Contributor

0xdavinchee commented Jul 19, 2021

Intro
I believe that refactoring the olympus-frontend to utilize Typescript will enable all current and future developers to have a nicer development experience and become much more productive in the long run as having types has multiple benefits: consistent styling and clear and explicit intent.

Coding on a team includes the dual task of expressing to the computer what you want it to do and communicating effectively to your teammates (and future self) what your intent is. This sort of "common language" is essential especially when trying to coordinate development efforts in such a decentralized manner, especially at scale. Below I outline the goals of this proposal, the outcomes I hope to achieve from it, the needs of the proposal, alternatives, a rough timeline and additional information for reference.

Goals

  • Discuss on a set of Typescript standards we will use for the refactor
  • Refactor the olympus-frontend to utilize Typescript
  • Ensure there are no breaking changes through rigorous testing, especially if we're planning on doing a project-wide refactor

Outcomes

  • Improve the developer experience
  • Some cleanup of unused props, etc.
  • Allow developers to work more efficiently allowing productivity gains across all developers once this change is implemented:
  1. TS will result in more readable code, intent will be explicit for both other developers and your future self.
  2. Productivity gains for developers: there is an initial cost (of time) where sometimes the first developer working on something may not know the type and will have to discover it themselves, but this will allow all future developers to be certain of what the datatype is beforehand and the time taken to test this adds up when done multiple times across multiple developers (saves dev time for more pressing tasks)
  3. Autocomplete for contract functions w/ Typechain as well as knowing exactly what certain things return and take in as parameters.
  4. Easier debugging/Fewer bugs - Typescript is pessimistic and won't compile if something is off, for example if you haven't handled a case where something might be undefined: https://github.com/0xdavinchee/olympus-frontend/commit/18bf6df0f2cbfad023507059d4aa88e16456f87a#diff-f3a239449bbebaac8df3a75ce26114955a8ef7a2b7f768ce0a83bdae7e8972b4L102-R136
  5. Future refactors - similar to the point above, if we want to make any project-wide refactors in the future, having a TS project will enable developers to do so with full confidence in knowing that any misspelled variables or other issues would be caught immediately (code won't compile).

Needs

  • Developers
  • Lots of testing (can never be too safe)

Alternatives

  • I'm not aware of any TS alternatives in terms of type safety/checking and I believe given the support/industry standards around TS in the development community that this is the best solution.

Timeline
I believe that this refactor is fairly straightforwards, honestly the most difficult part with TypeScript is often setting up the project to allow TypeScript files and the project didn't require too many changes to get TypeScript fully working. However, there is always the possibility we may encounter more problems as development starts. In addition, integrating TypeScript should not break anything as long as everything is compiling correctly. The most time consuming part would be the testing.

  • Optimistic: 1-2 weeks
  • Realistic: 3 weeks
  • Pessimistic: 1 month

Caveats

  • There may sometimes be a small upfront time cost for the initial developer implementing things when the type is unknown
  • There will be packages which don't have types and this will require us to declare the module in a .d.ts file to override Typescript complaining: declare module "@package/no-types-package";
  • Certain things require complex understanding of Typescript (for example in constants.js, the addresses variable has a pretty complicated type:
export type Nested = { [key: string]: string };

interface IAddresses {
  [key: number]: { [key: string]: Nested | string };
}

I have provided a solution, but it's not the cleanest way to do it. This could be fixed by fixing up the way this object is defined or figuring out the way of defining this type in Typescript.

Additional Info
I've gone ahead and added some stuff to the project config so that it works and made changes to a few files just to illustrate the power of Typescript:

@Zayen-X
Copy link
Contributor

Zayen-X commented Jul 24, 2021

I love it! How are we going to to integrate this with ongoing projects?

@0xdavinchee
Copy link
Contributor Author

I love it! How are we going to to integrate this with ongoing projects?

Hey so for integrating w/ ongoing projects, I think the best option would be to merge develop into the ts-refactor branch every time a new ongoing project is merged into develop and then refactor these new changes as they come.

I agree that the most important priority is FOHMO 3 and by doing it this way will be the smoothest way to roll out the TS changes. I believe we will be able to push out all the projects for FOHMO 3 and then merge this refactor branch into develop pretty much right after.

@dwjanus dwjanus added the enhancement New feature or request label Jul 24, 2021
This was linked to pull requests Aug 13, 2021
@brightiron
Copy link
Contributor

closing this since the Typescript refactor is complete

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants