flow-strict-readonly-spec.md

Proposal: @flow strict-readonly

Motivation

Much of our product code uses immutable data structures. This is largely because when working with React (particularly: props and Hooks), accidental mutation is a common source of errors ("why won't this re-render?").

Today, enforcing this immutability is something that happens via a mix of Flow types, ESLint rules, and code review. It results in hard-to-read types like:

type Props = $ReadOnly<{
  a: $ReadOnly: {
    b: $ReadOnlyArray<C>
  }
}>

This would be easier to read, and less code to write, if we could instead express it as:

type Props = {
  a: {
    b: C[]
  }
}

Additionally, because this is enforced via code review today, it leads to places where we forgot to make a type readonly. This can lead to painful migrations, and hard-to-understand type errors for engineers.

By enforcing this at the file level, we can:

1. Simplify product code, reducing the LOC you need to write to do something
2. Reduce the number of bugs in product code caused by accidental mutation
3. Consolidate enforcement, removing the burden of enforcing this in code review
4. Bring the benefits of immutability to the 95% of code that needs it, while making it easy for the 5% of code that needs mutability for performance reasons -- that can't use Immer or ImmutableJS due to file size constraints -- to use regular @flow strict
5. Reduce the cost of modifying code, by reducing the incidence of type errors caused by interoperating mutable and immutable data

Proposed features

1. A new flow_mode: OptInStrictReadOnly. To enable it for a file, use @flow strict-readonly. This flag enables readonly and strict_local for the file.

2. The flag causes Flow to parse types declared in a file as:

   1. A[] and Array<A> as $ReadOnlyArray<A>
   2. [A] as $ReadOnlyTuple<A>
   3. {a: b} as $ReadOnly<{a: b}
   4. Map<A, B> as $ReadOnlyMap<A, B>
   5. Set<A> as $ReadOnlySet<A>
   6. WeakMap<A, B> as $ReadOnlyWeakMap<A, B>
   7. WeakSet<A> as $ReadOnlyWeakSet<A>

3. If a file uses @flow strict-readonly, a Flow Lint rule errors when you use $ReadOnly types or (some) +/- variance markers.

Open questions

1. Should this apply to imported types too?
2. Should this be a composite flag (@flow readonly strict-local)?
3. Should this be exposed as a project-level Flow option too?
4. What's the right place to make this change: as part of parsing, as a normalization step before parsing, or as part of polarity checking?
5. Could we consider an alternative approach, where all files are readonly by default, and we have $MutableArray, $MutableSet, $MutableMap, etc. as trap doors?

Success metrics

1. Adoption in product code