gcanti / babel-plugin-tcomb

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

Caveat: tcomb must be requireable #139

Closed NervosaX closed 8 years ago

NervosaX commented 8 years ago

This is grinding my usage of this plugin to a halt: I use SystemJS, where "require" is not part of the ecosystem. Rather, ES6 import tags are compiled down to asynchronous System.register calls.

Is there no way that the require tags can be converted to ES6 imports before letting later plugins in the pipeline convert the ES6 to ES5 (to systemjs, to commonjs, however it must be done)?

gcanti commented 8 years ago

I should rephrase that paragraph, if you look at the tests this plugin actually outputs

import t from 'tcomb';
NervosaX commented 8 years ago

Hrm, I definitely get a "require" in my code that crashes the app.

The following code:

import type {ReactElementT} from "tcomb-react";

gives me the compiled output:

System.register("cdk/js/main.js", ["tcomb"], function (_export, _context) {
  "use strict";

  var _t, _tcombReact, ReactElementT;

  return {
    setters: [function (_tcomb) {
      _t = _tcomb.default;
    }],
    execute: function () {
      _tcombReact = require("tcomb-react");
      ReactElementT = _tcombReact.ReactElementT || _t.Any;
    }
  };
});

Edit: Just to be clear, I get a similar error when getting types from anything, including tcomb.

gcanti commented 8 years ago

Ah ok, the problem is with import type declarations, you are right, I'm using require. Let me see if I can replace require with an import declaration

NervosaX commented 8 years ago

Apologies, thought that was what the caveat was referring to. Thank you very much.

gcanti commented 8 years ago

Currently the plugin compiles

// ImportSpecifier
import type { A } from "types";

to

import _t from "tcomb";

const _types = require("types");

const A = _types.A || _t.Any;

and

// ImportDefaultSpecifier
import type React from "react";

to

import _t from "tcomb";

const _react = require("react");

const React = _react.default || _t.Any;

Would be ok to do this instead?

// ImportSpecifier case
import _t from "tcomb";

import * as _types from "types";

const A = _types.A || _t.Any;

and

// ImportDefaultSpecifier case
import _t from "tcomb";

import * as _react from "react";

const React = _react.default || _t.Any;
NervosaX commented 8 years ago

Looks perfectly sensible. I can test a branch if that is helpful.

gcanti commented 8 years ago

I can test a branch if that is helpful

Yes thanks. lib is commited in, you can test it out simply doing

npm r babel-plugin-tcomb
npm i gcanti/babel-plugin-tcomb#139
NervosaX commented 8 years ago

The following converts from

import type {ReactElementT} from "tcomb-react";
import type {IntegerT} from "tcomb";

to

System.register("cdk/js/main.js", ["tcomb", "tcomb-react"], function (_export, _context) {
  "use strict";

  var _t, _tcombReact, _tcomb, ReactElementT, IntegerT;

  return {
    setters: [function (_tcomb2) {
      _t = _tcomb2.default;
      _tcomb = _tcomb2;
    }, function (_tcombReact2) {
      _tcombReact = _tcombReact2;
    }],
    execute: function () {
      ReactElementT = _tcombReact.ReactElementT || _t.Any;
      IntegerT = _tcomb.IntegerT || _t.Any;
    }
  };
});

It also works correctly on one of my own declared file.

gcanti commented 8 years ago

Nice, I'm going to publish this fix

NervosaX commented 8 years ago

Great, thanks so much for that.

gcanti commented 8 years ago

Released in https://github.com/gcanti/babel-plugin-tcomb/releases/tag/v0.3.20