gcanti / babel-plugin-tcomb

Babel plugin for static and runtime type checking using Flow and tcomb
MIT License
482 stars 22 forks source link

tcomb crashes when there are circular "import type" #171

Open lwchkg opened 7 years ago

lwchkg commented 7 years ago

How to reproduce:

  1. Download the ZIP file: tcomb-example.zip
  2. Extract the contents into a directory.
  3. yarn install
  4. yarn compile (without tcomb)
  5. yarn execute
  6. yarn compile-debug (with tcomb)
  7. yarn execute.

Expected result Step 7 produces the same result as step 5.

Actual result Crashed. Here's the log (also in the ZIP file):

lwchkg@I7-4790 D:\Documents\pCloud Sync\github\tcomb-example                                               
> yarn compile                                                                                             
yarn compile v0.27.5                                                                                       
$ babel -d babel-out/ src/                                                                                 
src\a.js -> babel-out\a.js                                                                                 
src\b.js -> babel-out\b.js                                                                                 
src\index.js -> babel-out\index.js                                                                         
Done in 1.08s.                                                                                             

lwchkg@I7-4790 D:\Documents\pCloud Sync\github\tcomb-example                                               
> yarn execute                                                                                             
yarn execute v0.27.5                                                                                       
$ node babel-out/index.js                                                                                  
A =  A { b: null }                                                                                         
B =  B { a: A { b: null } }                                                                                
Done in 0.48s.                                                                                             

lwchkg@I7-4790 D:\Documents\pCloud Sync\github\tcomb-example                                               
> yarn compile-debug                                                                                       
yarn compile-debug v0.27.5                                                                                 
$ cross-env NODE_ENV=test babel -d babel-out/ src/                                                         
src\a.js -> babel-out\a.js                                                                                 
src\b.js -> babel-out\b.js                                                                                 
src\index.js -> babel-out\index.js                                                                         
Done in 1.41s.                                                                                             

lwchkg@I7-4790 D:\Documents\pCloud Sync\github\tcomb-example                                               
> yarn execute                                                                                             
yarn execute v0.27.5                                                                                       
$ node babel-out/index.js                                                                                  
D:\Documents\pCloud Sync\github\tcomb-example\node_modules\tcomb\lib\fail.js:2                             
  throw new TypeError('[tcomb] ' + message);                                                               
  ^                                                                                                        

TypeError: [tcomb] Invalid argument type undefined supplied to maybe(type, [name]) combinator (expected a t
ype)                                                                                                       
    at Function.fail (D:\Documents\pCloud Sync\github\tcomb-example\node_modules\tcomb\lib\fail.js:2:9)    
    at assert (D:\Documents\pCloud Sync\github\tcomb-example\node_modules\tcomb\lib\assert.js:14:12)       
    at Function.maybe (D:\Documents\pCloud Sync\github\tcomb-example\node_modules\tcomb\lib\maybe.js:24:5) 
    at Object.<anonymous> (D:\Documents\pCloud Sync\github\tcomb-example\babel-out\b.js:18:30)             
    at Module._compile (module.js:569:30)                                                                  
    at Object.Module._extensions..js (module.js:580:10)                                                    
    at Module.load (module.js:503:32)                                                                      
    at tryModuleLoad (module.js:466:12)                                                                    
    at Function.Module._load (module.js:458:3)                                                             
    at Module.require (module.js:513:17)                                                                   
error Command failed with exit code 1.                                                                     

lwchkg@I7-4790 D:\Documents\pCloud Sync\github\tcomb-example                                               
>

What caused the crash? index.js imports a.js, a.js "import type" b.js, and b.js "import type" a.js. There's a type statement in b.js, using a type in a.js, but the type is not available yet because this is a circular import.

lwchkg commented 7 years ago

Anyway, here are the contents of the source files:

index.js

// @flow
import {A} from "./a.js";
import {B} from "./b.js";

const a = new A(null);
const b = new B(a);

console.log("A = ", a);
console.log("B = ", b);

a.js

//@ flow
import type {B} from "./b.js";
export class A {
  b: ?B;
  constructor(b: ?B) {
    this.b = b;
  }
}

b.js

// @flow
import type {A} from "./a.js";

// The following line causes the problem. Removine the line and
// s/MaybeA/?A/ makes the file runnable.
type MaybeA = ?A;

export class B {
  a: MaybeA;
  constructor(a: MaybeA) {
    this.a = a;
  }
}
lwchkg commented 7 years ago

I have edited the generated b.js a bit and it seems to fix the problem:

var MaybeA = _tcomb2.default.maybe(_a.A, "MaybeA");

becomes a lazy evaluating function with closure:

var MaybeA = function() { var _ty; return function() { if (!_ty) _ty = _tcomb2.default.maybe(_a.A, "MaybeA"); return _ty; } }();

And of course

    _assert(a, MaybeA, "a");

becomes

    _assert(a, MaybeA(), "a");
gcanti commented 7 years ago

Flow allows for circular dependencies at the type-level. However babel-plugin-tcomb needs to reify those types at the value level so avoiding circular dependencies in the first place seems the best option.

I'm not sure that a lazy definition like that would handle all the use cases

lwchkg commented 7 years ago

Flow allows for circular dependencies at the type-level. However babel-plugin-tcomb needs to reify those types at the value level so avoiding circular dependencies in the first place seems the best option.

Sadly those circular dependencies are not always removable. Consider the case a parent object that owns a few children, and each children has a weak reference to the parent. This is a circular reference, but not a bad programming habit at all.

I'm not sure that a lazy definition like that would handle all the use cases. No. By adding export b_singleton = new B(null); at the end of b.js, the lazy evaluations will be immediately invoked and still crash. However, declarations alone will not be able to crash the lazy definitions, be it type, function or class.

Anyway, unless we can put the definition of A in another file, we can't really be sure that the definitions will be loaded before it is executed. But I don't think we can do this with babel.