babel-plugin-tcomb icon indicating copy to clipboard operation
babel-plugin-tcomb copied to clipboard

tcomb crashes when there are circular "import type"

Open lwchkg opened this issue 7 years ago • 4 comments

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 avatar Jul 30 '17 15:07 lwchkg

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 avatar Jul 30 '17 15:07 lwchkg

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");

lwchkg avatar Jul 30 '17 15:07 lwchkg

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

gcanti avatar Jul 30 '17 15:07 gcanti

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.

lwchkg avatar Jul 30 '17 17:07 lwchkg