frankwallis / plugin-typescript

TypeScript loader for SystemJS
MIT License
248 stars 47 forks source link

Catching type errors #130

Closed guybedford closed 8 years ago

guybedford commented 8 years ago

From this Tweet by @bennadel - https://twitter.com/BenNadel/status/734399795641655296:

can I enforce type-checking in the TS compiler in the NG2 Plunkr getting started? meaning if I have fn() : string but fn() returns Observable, I’d like to see an error.

@frankwallis do you know if this is possible with this plugin approach at all?

bennadel commented 8 years ago

Thanks for creating this - I'm really trying to get on the band-wagon :)

guybedford commented 8 years ago

Sure, it really helps to hear these questions to know where polish is needed!

frankwallis commented 8 years ago

I've updated this plunkr to turn on the type-checking, and add typings configuration for the angular packages and rxjs:

https://plnkr.co/edit/p1herPVQOcSKYq8YWXCd?p=preview

guybedford commented 8 years ago

Amazing, thanks! Hopefully we can get the Angular demos to update to this?

bennadel commented 8 years ago

@frankwallis @guybedford awesome stuff! After some fiddling, I'm able to get this working in my local demo. One question, though - what does this do in the import statement:

return System.import("plugin-typescript")
    .then(function(pts) {
        return pts.bundle();
    })

The demo seems to work with or without this statement. So, I'm not sure if it's actually needed.

bennadel commented 8 years ago

I don't want to overstay my welcome here, but I have a question about the expected behavior of type-checking. I have your demo working locally, but I have code that looks like this (sort of):

public values: Foo[];
// ....
function handleResponse( newValues: Bar[] ) {
    this.values = newValues;
}

Here, you can see I am declaring the values as an array of Foo. But, I get no errors when I try to assign an array of Bar to it. And, I'm wondering if that's because I need to have a custom typings file somewhere? Or, if this just isn't a check that the compiler will look at anyway.

Sorry if these are obvious things, I'm super new to TS.

guybedford commented 8 years ago

@bennadel these are both good questions. About the bundle, I'm not sure actually, perhaps that was just @frankwallis testing the behaviours. For the typing, perhaps @frankwallis can suggest further as well, I'm also not super familiar with typescript myself unfortunately.

bennadel commented 8 years ago

@guybedford I think that maybe the type-mismatch was working because the value being passed into the function was being returned from another function whose type matched. So, while I had handleResponse( newValues: Bar[] ), TypeScript new that it was actually receiving Foo[] based on something higher up in the call-stack .... so maybe it was just ignoring my method signature.

Only a guess :D

bennadel commented 8 years ago

FYI, I shared these results on my blog.

http://www.bennadel.com/blog/3095-better-type-checking-with-in-browser-typescript-transpiling-in-angular-2.htm

bennadel commented 8 years ago

FYI, that type-checking problem I was having was due to the fact that I was using method.bind( this ). Apparently .bind() kills type checking.

frankwallis commented 8 years ago

@bennadel thanks for the awesome blog post, I'm glad I could help get it working.

Regarding the bundle call - this is not strictly necessary, I used it in the plunkr to debug an issue I was having with the typescript definition file not being loaded so you can safely remove it.

bennadel commented 8 years ago

@frankwallis ah, good to know, thanks. And thanks again for all the help !