search

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

Fix rest params (#36) #37

Closed motiz88 closed 8 years ago

motiz88 commented 8 years ago

As the T in Array<T> is not checked right now, and the ...-to-Array desugaring is internal to Babel, I elected to remove the runtime check here entirely and convert it to a modest static check that the annotation indeed asks for an array type.

By the way, should this check be stricter and enforce that types be exactly ["array"]?

phpnode commented 8 years ago

@motiz88 this looks really good, thanks. Regarding "should this check be stricter and enforce that types be exactly ["array"]?" - yes, I don't think there are any circumstances where that isn't a mistake.

phpnode commented 8 years ago

@motiz88 thanks! I think your solution is correct, I'm imagining an annotation like this:

function demo (...args: Array<string>|Array<number>) {

}
motiz88 commented 8 years ago

@phpnode That's the thing, though. I thought so too at first, but extractAnnotationTypes() appears to remove duplicates, which would seem to make the check overly defensive given the current code. It's robust in the sense that it won't break if a types array with duplicates does slip through somehow, but that's really a matter of taste at this point, or rather, a question of whether extractAnnotationTypes() is guaranteed to dedupe its result.

phpnode commented 8 years ago

I think that will need some work in the near future anyway in order to support e.g. Array<number|string> properly