Skip to content

RFC: Introduce TypeScript to monorepo #4087

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

Closed
mcous opened this issue Sep 19, 2019 · 5 comments
Closed

RFC: Introduce TypeScript to monorepo #4087

mcous opened this issue Sep 19, 2019 · 5 comments
Labels
accepted For designating accepted RFCs RFC Software proposal ("request for comment")

Comments

@mcous
Copy link
Contributor

mcous commented Sep 19, 2019

overview

I know I've been talking about this (at varying levels of seriousness) for a while now, so I think it's time that I actually put all the things I've been saying into an RFC and see what happens.

  • A typing system for JavaScript is good
  • TypeScript is a typing system for JavaScript
  • Flow is also a typing system for JavaScript, but it’s really frustrating to work with
  • Maybe we should switch to TypeScript, which seems better than Flow in a lot of ways

I'm not trying to die on a hill or anything here, and choosing to keep Flow is a valid option here, but after a lot of time considering this, I think TypeScript is a better solution for our team than Flow. Switching is only ever going to get harder, so I want to (officially) start the conversation now

Please sound off in the comments if you've got any of your own for / against points. I'll start a thread in Slack for Q/A

TL;DR

Want to just see what it looks like before you read my essay below?

research and reasoning

To research this switch, I’ve been spending time building several new apps/libraries in TS that are part of a JS monorepo of mine. This has allowed me to try out the following:

  • What’s TypeScript like in a webpack + babel toolchain?
    • Answer: pretty much just works
  • What’s TypeScript like in a mixed-language repository with existing tools (mocha, eslint)?
    • Answer: pretty much just works
    • Caveat: non-trivial setup work required, but now I know exactly how to do it
  • How does TypeScript behave in a monorepo?
    • Answer: it’s ok, but support could be better
    • More specifically, it’s a little painful in a monorepo full of npm published libraries
    • Since our monorepo is mostly apps (where we can set package.json::main to a source file rather than a compiled file) we actually end up side-stepping most of that pain
    • Keeping an eye on Resolving multiple package.json "main" fields microsoft/TypeScript#21423
  • How are third-party libraries?
    • Answer: better than flow in every way, but not perfect
    • TypeScript provides mechanisms to easily work around the imperfections
  • How does TypeScript work with React?
    • Answer: Great! Zero problems
  • How does the syntax feel compared to Flow?
    • Not very different at all at the macro level
    • Close enough that I don't think we'll have any serious learning curve
    • Certain community preferences are different
      • e.g. prefer Element[] to Array<Element>
      • e.g. prefer interface to type for objects
    • TypeScript imports are first-class
      • i.e. import {SomeType} from ‘module rather than import type {...
    • & works just fine for combining object types, so you don’t spread

Reasons to switch to TypeScript

  • Lack of high-quality third party definitions in Flow, abundance in TS
  • Lack of high-quality first party definitions in Flow, abundance in TS
  • Flow is buggy with lots of breakages going from release to release, TS follows semver and keeps things stable
  • TS has a more convenient / flexible typing system
    • Note: this is at the expense of type “soundness”, which is a goal for Flow and a non-goal for TS
      • IMO type soundness is pretty academic when we need to ship code
    • Conditional types
    • Easy syntax for type-refining filters
    • Easier to work with enums (e.g. diff enums)
    • Easier to work with objects (e.g. exact objects by default, Pick and Omit types, easy mapped types)
    • More convenient syntax for property access:
      • Flow - $PropertyType<MyDef, ‘key’>
      • TypeScript - MyDef[‘key’]
  • Flow is slow to typecheck and report back to editors
    • Known: TypeScript editor support is fantastic and responsive
    • Unknown: is TypeScript faster with our codebase to check?
  • Libraries in our toolchain are written in TypeScript
    • Redux, Reselect, Formik, Jest (in progress), Electron (in progress), yarn v2 (in progress), electron-builder
  • Hiring: anecdotally, JS devs are more excited to work in TS than Flow

Reasons to keep Flow

  • Codebase inertia
  • Libraries in our toolchain are written in Flow
    • React, Babel, yarn v1 (current)
  • Better support for ES proposals like optional chaining (To be released in TS 3.7)

Next Steps

If we wanted to evaluate TypeScript in our monorepo, I think something like the following could work:

low commitment

To start, nothing we do should harm other projects, nor should it be difficult to fully reverse any changes.

  1. Introduce TypeScript into the toolchain alongside Flow
    • Babel
      • Should get us Webpack and Jest with minimal config changes
    • ESLint
  2. Port one of our more isolated projects from Flow to TypeScript
    • "Isolated" means minimal dependencies and dependents
    • Proposal: app-shell
      • Zero dependents
      • Depends on a small number of types from app which can be shimmed
    • Second proposal: discovery-client
      • Two dependents: app (types) and app-shell (functionality)
      • Could be done after app-shell, with types for app shimmed
  3. Evaluate and decide whether or not to proceed

high commitment

If we choose to proceed to after the low-commitment changes, then we get into territory where reversing course (or even just not moving forward quickly enough) becomes more painful than getting to full TS.

  1. Decide how to proceed with migration
    • Option 1: Continue to do one project at a time with type boundaries shimmed
      • Those type boundaries will wither require a lot of work or require allowing ourselves to leave an amount of code uncovered
    • Option 2: Move all remaining projects at once
  2. Actually switch to TypeScript according to (4)
    • I am confident we could do it in 0.5 to 1 sprint with two developers
@mcous mcous added the RFC Software proposal ("request for comment") label Sep 19, 2019
@IanLondon
Copy link
Contributor

I don't find Flow too frustrating to work with. In app, Flow takes a really long time to run its checks and something about app breaks Flow's cache when you make small changes, but that's the only persistent problem I have with it. I hit a few big bugs that required $FlowFixMe when it shouldn't have, but I haven't hit that in a year or so. Flow checker is slow to start up in PD/LL/shared-data but generally responds to updates in a reasonable time. I don't know that TS would be faster. The definitions for 1st/3rd party libraries and the better community support makes TS a better long-term pick IMO.

If we get stalled with product work and have a free sprint that we can't spend on other tech chores, I think this would be a good move forward. But things like: RPC removal, finding an alternative to Travis that doesn't bottleneck us and has a manual confirm feature (for terraform deploy later), and implementing the atomic command layer as the sole interface on top of hardware controller are all more urgent projects with more immediate benefits than Flow -> TS switch.

I'm OK with the high commitment path (& option 2), the low commitment path is dangerous for the reasons you outlined, and option 1 where we leave things uncovered is scary. I just would rather have us work on other problems in the next couple months, this isn't a pressing one for me personally.

@mcous
Copy link
Contributor Author

mcous commented Oct 2, 2019

Thanks for the thoughts @IanLondon!

I don't know that TS would be faster.

Yeah, this question is a little hard to evaluate. All we have is anecdotes, but in every single anecdote I have (including two that are my own anecdotes) TypeScript is noticeably faster and more responsive.

The definitions for 1st/3rd party libraries and the better community support makes TS a better long-term pick IMO

I wonder if we have team consensus on this statement. If so, I think it is worth noting that switching to TS only becomes harder over time. If we don't switch now (e.g. before we add another four devs to the team), we might never. If our long-term goal is, in fact, to use TypeScript, we need to commit sooner rather than later.

I just would rather have us work on other problems in the next couple months, this isn't a pressing one for me personally.

Things make this "pressing" for me:

  • Flow is slow and we have a lot of uncovered code that most of us aren't aware of
    • What use is a type system that isn't protecting us?
  • Flow breaks stuff every time we upgrade it, and we need to upgrade it because Flow is buggy and misses type bugs unless we upgrade it
    • What use is a buggy type system that involves (me) sinking a bunch of time into upgrading it to fix its bugs?
  • I sink a lot of time into writing and maintaining buggy flow-typed libraries

But things like: RPC removal, finding an alternative to Travis that doesn't bottleneck us and has a manual confirm feature (for terraform deploy later), and implementing the atomic command layer as the sole interface on top of hardware controller are all more urgent projects

Of the other projects you listed, they're definitely all important, and in terms of having to rank stuff, they are more important. But aside from CI changes (which I don't think are fully scoped, so I personally wouldn't put them on this list as urgent) and RPC removal, I don't see how they are affected, because they don't really share developers or codebase.

In terms of RPC removal, I would much rather implement the client side of the Sessions API in TypeScript rather than Flow. It makes me think of our proactive introduction of redux-observable in the app when there was "more urgent" feature work, but our tech debt pay-down allowed us to get the buildroot front-end in place in weeks instead of months

@IanLondon
Copy link
Contributor

we have a lot of uncovered code that most of us aren't aware of

@mcous uhhh yikes that sounds bad! what do you mean?

I would much rather implement the client side of the Sessions API in TypeScript rather than Flow

Is there a difference particular to this project?

@mcous
Copy link
Contributor Author

mcous commented Oct 3, 2019

As discussed in person:

we have a lot of uncovered code that most of us aren't aware of

@mcous uhhh yikes that sounds bad! what do you mean?

I should've included more nuance in this. There's a couple things here:

  • Bugs (or "bugs") in flow itself make things appear covered when they are not really
    • I've seen this happen mostly at component boundaries, especially if inexact prop types are involved
  • More common: due to the generally lower quality of flow editor tooling, we disable (and/or have disabled in the past) the editor options to display flow coverage
    • This leads us to a world where a lot of our code, and sometimes even new code, is written and we deem it "safe" because it passes flow
    • However, with coverage reporting enabled, we see a bunch of anys and nevers and see that the code isn't covered at all

I would much rather implement the client side of the Sessions API in TypeScript rather than Flow

Is there a difference particular to this project?

The main difference is that it will be a relatively greenfield project within the app that will likely involve:

  • Smarter organizational patterns around file structure
  • Smarter organizational patterns around redux state
  • New (or better use of existing) dependencies for API interaction

Given TypeScript's higher quality third-party library definitions (in this case, for redux and whatever else we pull in), I suspect the implementation it'll be easier / better with TypeScript than with Flow

@mcous mcous added the accepted For designating accepted RFCs label Jan 17, 2020
@mcous
Copy link
Contributor Author

mcous commented Jan 17, 2020

We're moving forward with introducing TypeScript into our codebase in a considered manner that does not involve the monorepo at first. If and when we get to the monorepo, we'll have new tickets to track that work. Closing this ticket accordingly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted For designating accepted RFCs RFC Software proposal ("request for comment")
Projects
None yet
Development

No branches or pull requests

2 participants