codemix / babel-plugin-typecheck

Static and runtime type checking for JavaScript in the form of a Babel plugin.
MIT License
886 stars 44 forks source link

Transformed async function throw an error with typecheck #104

Closed tp closed 8 years ago

tp commented 8 years ago

When using "transform-async-to-generator" and "transform-regenerator" together with typecheck I get an exception at runtime (see below).

When removing "transform-async-to-generator" everything runs fine so far. Maybe that plugin was not needed, then I would just leave this ticket as a future reference.

Do you think I had a config error before, or should this have worked?

.babelrc:

{
  "presets": ["es2015", "stage-2"],

  "plugins": [
    "syntax-class-properties",
    "transform-class-properties",
    "transform-es3-property-literals",
    "syntax-async-functions",
    "transform-async-to-generator",
    "transform-regenerator",
    "transform-runtime",
    "syntax-decorators",
    "transform-decorators",
    "syntax-function-bind",
    "transform-function-bind",
    "syntax-flow",
    "transform-flow-strip-types",
    "typecheck"
  ]
}

testcase.js:

#! /usr/bin/env node

require('babel-register');

require('./testcase2.js').default();

testcase2.js:

/* @flow */

async function test(): Promise<boolean> {
  return true;
}

export default function() {
  test().then(res => console.log('test():', res));
}

Output:

=> ./testcase.js
/Users/timm/Projects/node_modules/babel-core/lib/transformation/file/index.js:548
      throw err;
      ^

SyntaxError: /Users/timm/Projects/testcase2.js: Function "test" returned an invalid type.

Expected:
Promise<bool>

Got:
true
  2 |
  3 | async function test(): Promise<boolean> {
> 4 |   return true;
    |   ^
  5 | }
  6 |
  7 | export default function() {
tp commented 8 years ago

Under more complex circumstances I can get this to fail when using return true; even without the transform-async-to-generator plugin. Will try to extract another test case, but maybe you already have a hunch.

tp commented 8 years ago

Also changing the return type from Promise<boolean> to just boolean (which works fine form Flow's perspective IIRC) does not help.

phpnode commented 8 years ago

@tp please place the typecheck plugin before the others in the list in your .babelrc, and then try again. Babel 6 is extremely sensitive to plugin ordering.

However I think that this is happening because of a mismatch in flow and typecheck's semantics around async functions - see this issue: https://github.com/facebook/flow/issues/1259

I bet this will work:

async function test(): boolean {
  return true;
}

I actually fixed this problem locally but haven't published it yet. I'll try and find time for that tonight.

tp commented 8 years ago

Strangely now it works for me either way with Promise and boolean. No changes to the config or plugin.

What kind of change do you have pending locally?

I also think that just annotating the return function without the promise wrap is clear enough since the whole function is async, but if flow requires the Promise we will have to stick with it.

phpnode commented 8 years ago

Pending change accepts Promise<foo> and foo as identical in async functions. That's how I feel too but Flow goes the other way and compatibility with Flow is becoming more important to me now that it is supporting more and more syntax. The plan is to reach Flow compatibility in v4.

tp commented 8 years ago

Ah, I just remembered how to get it broken: annotating as returning Promise and just using a plain return X at the end of the function.

tp commented 8 years ago

Any way you could share that pending change as a development branch? Would really love to not add any more return Promise.resolve(X) if those can be avoided.

phpnode commented 8 years ago

@tp yes, sorry I meant to do it a day or two ago but a bit busy right now. I'm not near that machine at the moment but I'll push the branch or just do a patch release later today (I've added a reminder for this).