Reach full compatibility with flow

[fixplz]

So there is the choice when an unsupported feature or annotation is encountered between ignoring and showing a warning (I didn't want to investigate if there is a non-error way of showing a message through Babel so I opted for throwing).

Silently ignoring annotations has the downside of being confusing for users. I don't know what different people look for in this plugin but likely many will want something simple and predictable without having to learn how Flow works. If we cater for this case then it's important to warn users about cases where no type check will be performed despite an annotation being present.

Flow rejects language features it doesn't support - the idea is to provide a very good guarantee that the program will work or give an error. So you can't use this plugin together with Flow to cover cases it doesn't support. If a Flow check succeeds then there will be no type error exceptions of the kind our plugin produces.

From our side silent failures are contrary to how Flow is meant to work.

So I want to understand your aim regarding this - how should this be compatible with Flow?

[phpnode]

I want people to be able to use all of the annotations which flow supports, with the view that this plugin will support those constructs in future. We should never prevent the programmer from using something which is valid flow syntax.

Since we can't support everything all at once, let's introduce an environment variable, TYPECHECK_STRICT which throws on unsupported constructs. We could log unsupported annotations to stderr by default, or introduce another env var - TYPECHECK_VERBOSE which turns that on/off

[phpnode]

3.0.0 now supports just about all of Flow's features, closing this for now until specific examples crop up.

[fixplz]

We still generate broken code for things like:

function each<X> (arr : Array<X>) {
  arr.forEach((el : X) => f(el))
}

I added an error for this case before, because it's better than trying to generate code for it or silently ignoring it.

I'm trying to understand if you're saying we should allow users to use things like f<X> : (Array<X>, ...) => ... and just ignore them.

I've looked over what you added for v3 (great work!); the fact remains that this plugin can only do shallow type checks and we can't support all Flow features (like polymorphic functions or arrays with a type param).

We should by default reject Flow syntax and features we don't/can't support.

The job of a type checking system is to generate guarantees. We need to translate every annotation into a guarantee. If we can't, we should reject the annotation.

(Like you said, there could be a config to disable errors on unsupported features. Though I think if someone wants to use Array<X> as a form of documentation they should write Array/*<X>*/)

[phpnode]

(we do now support arrays with type parameters, we generate an arr.every() call, we do something similar for Map<string, number> and Set<string>).

It's true that we don't produce checks for polymorphic types, there is a way to do it but it's extremely ugly. I don't want to prevent or discourage people from using those annotations though. Polymorphism is a fact of life in JavaScript, people need to be able to annotate those things so that they can also use flow and to communicate that information to other humans. The annotation is still valuable, even if the check is not enforced.

I don't want it to be a default, but since Babel now has a way of specifying plugin options, please feel free to add an optional strict mode which enforces this. We should probably also add a mode which warns rather than failing to compile.

We can also do a better job of explaining the limitations in the README.

[fixplz]

Alright, I'll see how it works out for my team.

I see I misunderstood the behavior of the type variable above. Though we still have a problem in that the inner el : X generates a check even though the X refers to the type variable.

[fixplz]

Implementing type variable checking could potentially be easy: generate a stateful check function when a variable gets introduced to do "runtime unification"

var X = typeVarChecker()
if([...] arr.every(X))

But let's not

[phpnode]

yeah i worry that the contortions we'd need to go to to support that are just not worth it, particularly for classes. We should be able to reliably track references to polymorphic types though so please file an issue if it generates the wrong check for the example you provided.

[phpnode]

This was impossible without a runtime library, however it is the goal of https://codemix.github.io/flow-runtime