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

Support for eg Array<string>? #10

Closed ShardPhoenix closed 9 years ago

ShardPhoenix commented 9 years ago

Hi,

Flowtypes apparently supports annotations like Array, which checks that only string values are included in the array. Currently this plugin ignores the type parameter and just checks that the value is an array (so eg you can pass ["abc", 1] to a function that takes Array). Are there any plans to support type parameters?

phpnode commented 9 years ago

@ShardPhoenix I'd like to but there's a tension here because the typechecks need to be fast and small enough that people can leave them enabled in production if they want to. Right now they incur almost zero overhead. This feature, along with things like type aliases are probably too slow for many production systems. Probably there needs to be a way to set the strictness level, but that would be blocked by https://github.com/babel/babel/issues/1833

ShardPhoenix commented 9 years ago

Thanks for the response. I can understand why performance might be an issue. It would be good to go with your idea for a strictness level option if and when possible.

cesarandreu commented 9 years ago

People truly leave typechecks enabled in production?

phpnode commented 9 years ago

@cesarandreu why not? The performance impact is laughable in 99.9999% of cases and it can protect against things that testing didn't catch. Of course this won't work for everyone, but most projects can safely leave them on.

timoxley commented 9 years ago

@phpnode I've seen runtime type checking add severe negative impact to webpage performance, the difference between usable with them off and not usable with them on. It entirely depends on how much data your app needs to crunch and it's not uncommon for apps to already be running on a tight performance budget without any additional laughable overheads.

phpnode commented 9 years ago

@timoxley you've seen dramatic performance impact from using this plugin in particular or typecheck libraries in general? I can see this causing performance issues when the expansion causes the function to exceed V8's max-inlined-source-size threshold, or in very very tight loops, but we're in micro optimization territory - typeof and instanceof are fast!

That being said, I totally agree that there are scenarios where this matters, and I definitely want a nice way to turn them off in production - waiting on babel for this at the moment. But the fact is that most apps are not written for speed at all and those apps will not generally suffer from leaving this turned on.

timoxley commented 9 years ago

@phpnode I've seen dramatic performance impact from a bunch of typecheck libs. Was considering trying this out, and being able to toggle the checks on and off easily is a big +1 for peace of mind knowing I'm not locking myself into a decision too tightly.

phpnode commented 9 years ago

@timoxley we could look for an environment variable and make the plugin a no-op if it's set, e.g.

DISABLE_TYPECHECKS=1 babel ./src
phpnode commented 9 years ago

@timoxley for a number of reasons this plugin should be a lot faster than most typecheck libraries, I'd encourage you to see for yourself :)

By the way, there's no lockin - just remove the relevant line from your .babelrc and everything will work as normal but without typechecks.

STRML commented 9 years ago

FYI, in your .babelrc, just do something like:

"env": {
  "development": {
    "plugins": [
      "typecheck"
    ]
  }
}

if you're concerned about performance (I am, until I am sure this doesn't slow us down).

jthomaschewski commented 9 years ago

EDIT: Sorry, just saw that I posted this comment in the wrong issue...