Olena Sovyn

Posted on Dec 13, 2019 • Edited on Jun 21, 2020 • Originally published at frontendgirl.com

Flow type best practices

#flow #flowtype #javascript #types

Current, Flow is not released even in version 1.0.0, but it is already flexible enough to allow creation type systems in different ways. I decided to share a set of rules, that from my experience are making types system based on Flow more stable, reliable, predictable and useful.

Why add Flow to the project

1. Refactoring freedom

When flow annotations are added to the files, it looks like the right thing to do, but it fully shines when one decides to refactor component or function, as there will be no need to keep in memory where the component or function was used. For example, one has changed the type of the arguments, that utility function will accept as arguments. If the project has sufficient flow coverage, logs of flow errors will lead to all places where this change will require changing the arguments with which function was called.

2. No more unused arguments or properties

With keeping flow annotation as strict as possible it is easy to detect argument (in the function calls) or properties passed to the React components, that are never used. This enables removing them more easily and safely. And even though that existence of this code on the surface doesn't cause any trouble, but every unused argument/property is a potential source of hours of wasted engineering time, who will be trying to figure out is it used or not (especially if in many places arguments or properties are passed using spreading functionality), and moreover at the end of these hours even more time wondering if these are not used, is it really safe to delete them. The existence of the strict flow annotations can save all this time.

3. Documentation at place

Having flow annotation, especially for the function, classes, and variables, that are used outside of the module is in many ways a similar way for how function and classes were previously having inline documentation with JSDoc. For example, flow annotation for the utility function allows us to know what argument type function is expecting and what will be the type of returning value. Even more with the freedom that flow generic types are providing. We can have a better way to describe what argument types are accepted and what will be the return value. For example, for function:

const makeArray = (a, b, c) = [a, b, c];

there is no strict way to provide documentation about function return value if types of the a, b & c are not restricted, with the flow we can do something like:

const makeArray = <A, B, C>(a: A, b: B, c: C): [A, B, C] => [a, b, c];

const newArray = makeArray(3, 'test', false);

(newArray[0]: number);

(newArray[0]: string);

Open example in Flow Try playground

So instead of explicitly telling what the return value type should be, we have the freedom to provide this information based on the types of the function arguments.

How to make flow more efficient in the project

1. Add broad types only intentionally.

Starting from the v.0.88 Function and Object types are aliases of the any type, so all the things like following, will not cause errors:

const someObject1: Object = 3;
const someObject2: Object = true;
const someObject3: Object = 'some string';

const someFunction1: Function = 3;
const someFunction2: Function = true;
const someFunction3: Function = 'some string';

Open example in Flow Try playground

As you can see, there are no errors. Not exactly what one would expect reading the code on the left part and without knowledge, that Object is alias for any type.

However, this practice is not only about Object or Function annotations but about other types as well. Let's talk in more detail about specific cases of how this rule can be applied.

1.1 Object type

In most cases, the object has a defined set of properties that can be accessed on the objects.

var someObject: {|
  prop1: number,
  prop2: number
|} = {
  prop1: 4,
  prop2: 2,
};

Open example in Flow Try playground

Notice, that someObject here is annotated as the exact object. In general flow team, a while ago, has defined that they will be moving forward with making object type to have exact annotation by default: https://medium.com/flow-type/on-the-roadmap-exact-objects-by-default-16b72933c5cf. I would say that this is a move in the right direction, as the current case, when object annotations are not exact by default can lead to some confusion situations. About one of which that occurs on the attempts to spread object annotation, I wrote a little bit more a while ago (check "Inexact vs exact flow object types: spreading"). This case inspired me to add a new rule spread-exact-type in eslint eslint-plugin-flowtype.

Until we have object annotations exact by default I encourage you by default to annotate all objects as exact and leave inexact object annotations for exceptional cases only.

1.2 Any and FixMeAny

Let's be honest - annotating our code can be really hard, especially when we are talking not about shiny new features that were added last month, but some quite old code, that even though that was written ages ago is still actively used across the platforms. Nuanced mental models for such pieces might be lost. With remembering the rule, that "some types are better, than none" it does look like annotating some of the values, that have unknown/undiscovered types as any might be a good idea. However, when we look at the magic ball that will show us our future we might see pitfalls in this idea. If another person will see type any next to some value, they would have no idea, that this variable shouldn't actually have any type, but rather the type, that should be improved in the future.

One of the ways how this situation can be improved is adding separate suppression types, that can be used specifically for such cases - when currently we don't know what type we should write there, but it should be changed and made more strict in the future. This can be done in the .flowconfig as following:

Read more about suppress type: https://flow.org/en/docs/config/options/#toc-suppress-type-string.

This solution is not ideal, and even this specifically created type shouldn't be overused, but still, it is a nice and easy way to indicate that this type should be improved in the future effort.

1.3 Strings

Frankly speaking, this was the most surprising fact, that I found out when I was learning flow basis by myself

If the string value is not cast its types is string and not the exact string value

Something like following will not cause any errors:

var TAB_CLICKED = 'TAB_CLICKED';

('OTHER_ACTION': typeof TAB_CLICKED);

Open example in Flow Try playground

If we want the flow to show an error in this case, we would need to cast strings by one of the following ways:

// Way #1
var TAB_CLICKED = ('TAB_CLICKED': 'TAB_CLICKED');
('OTHER_ACTION': typeof TAB_CLICKED);

// Way #2
var TAB_CLICKED: 'TAB_CLICKED' = 'TAB_CLICKED';
('OTHER_ACTION': typeof TAB_CLICKED);

Open example in Flow Try playground

One of the most prominent examples, where string variables should be cast is action types in the Redux realization.

My suggestion is that all strings should be cast, the type of the variable should be left as string only in the case when it is really can be any string.

Bonus point:

I am currently removing Object and Function type from our code base, and ammount of flow errors caused by this drives me crazy. Is there any way, how can I do it more efficiently?

I will share with you my experience around this problem in small manageable steps:

When we have heard, that new release will make Object and Function be aliases of any, I have enabled no-weak-types in eslint-plugin-flowtype (https://github.com/gajus/eslint-plugin-flowtype#no-weak-types). This made the problem more visible.
After that, we have added per-line suppression comments to errors caused by this eslint rule, so when whoever will be working in that part of our codebase they will see that comment and would be able to fix the problem, that was suppressing. Notice, that this eslint rule can be configured in the way, that errors will be shown only on the Function and Object type, so any type may remain valid. If some values should really have any annotations, they can still have it.
When talking about removing types Function and Object to fix the initial problem, I would suggest trying to annotate value as strict as possible, but keep in mind, that some types are anyway better than none.
If you have completely no idea what type should be instead of Object go for the strictest object annotation {||}, then if the usage of this value is expecting some of the properties to be present in the object you'll get errors for them, and can add them this way incrementally figuring out them one by one.
If there is a need to remove Object annotation and we know that value should be annotated as any object, use annotation {} for this.
If you have completely no idea what types should be instead of Function go for the strictest function annotation () => void. This way if the function should be called with any arguments or/and should return some values you'll be notified by the flow errors about this, and would be able to fix annotations incrementally.

This rule has 2 sides: if to follow it and don't have full flow coverage over the codebase there is a big chance, that some of the values will be typed wrong, and will start causing errors when flow coverage will increase, as they might for example not encounter all properties that object should poses. It might look like the better way is to annotate that values less strict, so they would not fail for this future possible cases, but I don't agree with this way, as it leads to having too wide types in the codebase, that would potentially hide errors, that can affect business or UI logic. Better I'll get flow error in the code editor while I'll be changing or refactoring something, than error or even bug in production.

2. Have a naming type convention

It does look like this rule can have only a minor influence on the reliability of the flow type system in the project. It even might look like talking about this is only bikeshedding, but let's think about this a little bit longer and especially what will happen if we would have some rules about naming in place. For example, we have React based project with classical architecture - components, stores, action creators. When we would need to import somewhere type for the store state and have a naming convention in place we can do this even without opening store file. We might go for the convention, that store state types should always be named State and stored in the same file as a store, or it should be named ABCState for ABCStore and stored separately in the ABCState.js file. No matter with what convention team will run, but it does matter that they have the convention. It is not only makes importing types easier but also partly remove mental overload at the moment when types are created, as an engineer wouldn't need to make a decision how to name type, as well as for someone who'll read this code afterward it would be easier to understand what type variables are describing.

3. Reuse types when it makes sense

Let's think about the classical example of the web app - blog, and specifically about blog postentry object. One of the fields in this object is id, and we might go with annotation for this field to be string. However, is really any string an acceptable value in this field? Probably, no, as we are expecting there only strings, that were added to the blog entries as id to be valid strings. How could we better communicate this? I would suggest, that naming this type can be a right first step, so instead of id be annotated as string, we can do something like this:

Now when we would reuse type BlogEntryId in other parts of our application we would instantly provide knowledge, that at that place not every string can be accepted, but only one that came from blogEntry.id.

Using opaque type aliases for such cases might bring even more value. Read more about opaque type aliases: https://flow.org/en/docs/types/opaque-types/

4. Avoid "black holes" in the types

Let's start with sharing understanding what exactly I am here calling "black holes". So in my determination:

Black holes are functions return values of which have type any, even though their types can be more specific based on the function internal logic.

Judging from my experience there are 2 main reasons for such functions to appear in the codebase:

The function which return value was explicitly typed as any , for example:

const postCount = (posts: string[]): any => posts.length;
const posts = ['post1', 'post2'];

(postCount(posts): string);

Open example in Flow Try playground

The function is imported from the file, that hasn't been typed.

I don't think, that there is currently any way to explicitly prevent the behavior described in the first reason, however, I think, that potentially rule for this can be created in the eslint-plugin-flowtype.

Talking about the second behavior, it can be prevented by adding the next one configuration in .flowconfig.

Read more about untyped-import: https://flow.org/en/docs/linting/rule-reference/#toc-untyped-import

If you are working with React-based project I would suggest especially look-up for such behavior for HOCs (Higher-Order Components). Sometimes it can be very frustrating to properly annotate them, but at least adding specific type for the return value can help to start solving this issue. Frankly, speaking annotating HOCs can be a great mind gymnastic, at least it was one of the most challenging and interesting flow-related tasks for me.

5. Some are better than none

All previous points were about how to annotate in the most beneficial way for your codebase. Let's, however, keep in mind that all these are suggestions and not the restriction. Following them can make your type system more robust and bring more value. Though the next one is always true:

Some types are better than none

When one is starting adding types to project it can be a frustrating experience, do your best as there is no perfect. One of the techniques of how, for example, add an annotation to the complex object types is to add all the properties in the annotation and make all of them be the type FixMeAny (or equivalent suppression type in your codebase). This will allow you to work out what properties types should be one by one, and not feel like there are too many flow errors in the project at once, that you wouldn't be able to fix them. If some of the annotations will still be confusing to write, or they should logically be imported from the files that haven't been created/annotated yet and the scope of current PR is already too large, feel free to commit and merge as is, as FixMeAny is enough sign of the fact that this annotation is not a finite expected annotation.

6. Add types with cycles dependencies in your mind

At Webflow we have a huge codebase, and when we added a sufficient amount of type annotations in it we have noticed that the flow server started at some point to perform rather slow. One of the hypotheses some of our engineers had was that it can be affected by the amount of the cycle dependencies, that are present in the code.

So what are these cycle dependencies anyway? Cycle dependencies are created when file1 is importing some types from the file2 and file2 is importing some types from file1. For example, if we have ABCStore.js and it has type FileType imported from XComponent.jsx, and XComponent.jsx has type ABCState imported from ABCStore.js.

I don't know any 100% guarantee methods on how to work through this issue. One of the possible solutions might be keeping types in separate files, as this will allow not to import files that contain other possible exportables in the same file. For example, in the case described above it might look like:

Also, a good idea might be to have a tool on CI (we actually have something like this in Webflow), that will detect when new cycling dependencies are added on the branch (in comparison with the master or dev branch) and somehow prevent merging before the author will solve this problem. It is not exactly a solution, but at least this allows us to scope the problem and not allow it to blow up in the even larger amount of cycle dependencies.

If you have been observing similar problems in the project that you are working on and have found some other solution, please, bring them in the comments. I would love to learn from you.

7. Type errors in Unit tests

One of the question, that might appear in one's head is:

Should we have test for types themselves?

And even though it may look slightly over-engineering at first glance, but when we are introducing some more complex annotations, especially one, that are using generics in their logic, it may be a really good idea. With type casting expression it is possible to test complex annotation when passing arguments with different types. Also often in the unit tests function might be called with arguments, that are not valid for it to tests if the proper behavior will appear in this case. If to introduce separate suppress comments, like $FlowExpectError, it can be a great way to test was annotation for the function done properly as well, as if there will be no error at this place, flow will alert us that this comment is redundant. Different suppress comments can be added in the .flowconfig in the [options] part. Read more about how suppress comments can be configured https://flow.org/en/docs/config/options/#toc-suppress-comment-regex. For example, to add suppress comment as one I mentioned before ($FlowExpectError) we would need to add the following:

How to fix all already existing flow errors?

So, maybe you already have some flow annotation around the codebase and want to increase flow coverage in your project. And some question like this can appear in your head:

Do we need one folk that will just start fixing everything and adding new annotations and types?

"Yes" can be an answer for some teams and projects. Some others might opt-out for going in the way when adding annotations and creating types is everyone's business.

Some of the technique, that we at Webflow found useful for gradually increasing flow coverage, without making it a primary task for someone specifically:

Make sure that the project has a working and reliable eslint. Consider adding meaningful rules from the existing eslint plugin created to cover flow types, for example, eslint-plugin-flowtype. Also, have eslint fail if any errors are found on the CI, and prevent branched from being merged in the dev or master branch. This will not only prevent the spreading of the flow-related errors but also other errors, that can be detected with eslint.
When making rules to be more strict, concentrate first of all on preventing new errors, that will break new rules to appear rather than on fixing old one errors. On the moment of adding rule suppress all errors, that this rule detects with comments that will clearly indicate, that this should be fixed. This will allow fixing these violations gradually instead of the deep diving in the lake of the frustration of fixing hundreds of errors in the different parts of the codebase. I used these technics when I was adding the rule no-weak-types to prevent using Function and Object types. It allowed us to prevent future usage of these types. Also, this way may be useful, when upgrading to a newer flow version.
Deprecate adding new js/jsx files without flow pragma (this can be done with Danger.js). Annotating new functionality is much easier as it doesn't require rebuilding the mental model of what going in this part of the functionality.
Adding a warning on the CI level when the file that doesn’t contain flow pragma has been modified (also can be done with Danger.js). The logic of this technique is that if a person has modified files, they probably already have some part of the mental modal related to this file in their head, so they can be a good candidate to add flow coverage there.

Other cool things about adding flow

Frankly speaking, when I only started learning the flow and added it in a few of my PRs, I didn't have any special feelings about it. As my confidence and understanding of it grew, I started loving creating PRs that were all about flow annotations only. The reason for this is pretty simple - those PRs don't require QA-ing, as they don't change application logic, so they can be merged faster, and I can have my dopamine dose of getting something done :)

Post originally was published: https://frontendgirl.com/flow-best-practices/