Mobx Flow functions are not type checked by FlowType without .bind(this)

Question

When we use MobX flow decorated functions, flowtype thinks this is of type any and does no type checking.

class MyComponent extends Component<*> {
   actionMethod = flow(function*(){
        yield this.notAMethod();
   });
}

However, if we bind this to the generator, flow understands the type of this is MyComponent.

 class MyComponent extends Component<*> {
       actionMethod = flow((function*(){
            yield this.notAMethod();
       }).bind(this));
    }

My question is how can we ensure that these methods are type checked.

Is there any linting rule to ensure we bind(this) correctly?
Can we improve the mobx typings?
Is this an issue with flow?

Daniel · Answer 1 · 2018-10-12T17:45:35.763

this is derived dynamically in javascript. So flow can not tell you what type this will be with static analysis. The only exception is when you bind it explicitly to this (like you did in your second approach) or when you use an arrow function (but arrow functions don't work with yield). With that knowledge in mind, I will answer your questions:

Is there any linting rule to ensure we bind(this) correctly?

Flow tells you the type when using bind. So you don't need a linting rule, as the type is correct.

Can we improve the mobx typings?

It's not a mobx typings mistake. It is how javascript works. You can't tell what type this will be during runtime (except you use bind or an arrow function or call a method of an object's instance)

Is this an issue with flow?

See answer above.

Here is a long post about how this in javascript works https://stackoverflow.com/a/26574449/2379376

I hope I could help to clarify your question.

If you remove the call to flow, flowtype does know the type of this. Developers may forget to correctly bind this, so I guess the question is how do you lint against this being any. — Adam Mills, Oct 14 '18 at 07:27

score 2 · Accepted Answer · edited Oct 19 '18 at 01:04

Your question reduces to the ability to create a custom lint. Eslint has a great library for writing your own eslint rules. You can compare using https://astexplorer.net/, the difference between your code with and without this. Then you could write an eslint rule that looks something like this (Untested):

// @no-flow
"use strict";
// Help from  https://astexplorer.net/
module.exports.rules = {
  "mobx-flow-requires-bind": context => ({
    CallExpression: function(node) {
      const ancestors = context.getAncestors();
      const parent = ancestors.length > 0 && ancestors[ancestors.length - 1];
      if (node.callee && node.callee.name === "flow") {
        if (node.arguments.length === 1) {
          const callee = node.arguments[0].callee;
          if (!(callee && callee.property && callee.property.name === "bind")) {
            context.report(node, "Calls to flow must be used with .bind");
          }
        }
      }
    }
  })
};

You can integrate the above code into your repo with this tutorial.

score 1 · Answer 3 · answered Oct 16 '18 at 15:05

1

Here is a suggestion I was able to find in my research. Give it a try and let me know if it solves the problem. Basically they assigned the type of this manually.

public fetchProjects = flow(function*(this: ProjectStore) {
    this.projects = yield ProjectApi.get()
})

answered Oct 16 '18 at 15:05

Swazimodo

915
1
10
29

Thanks, I beleive this syntax is typescript only. – Adam Mills Oct 18 '18 at 22:33

Mobx Flow functions are not type checked by FlowType without .bind(this)

3 Answers3