nikku / karma-browserify

A fast Browserify integration for Karma that handles large projects with ease
MIT License
321 stars 50 forks source link

tsify compilation errors prevent bundle from being built #190

Closed bengro closed 8 years ago

bengro commented 8 years ago

I am using karma-browserify and the tsify plugin.

I have noticed that when I run the tests and there is a typescript compilation error, the bundle is never created and the tests won't be run. When there are no compilation errors the bundle is created and the tests run.

Whilst it should be encouraged to not have compilation errors, I've run into cases where I would not like my tests to not be executed just because there is a compilation error. Therefore, I would like to have an option in the settings, which allow me to bundle my typescript despite there being compilation errors.

I have quickly looked into the code I could discover the following:

  1. tsify fires error event, which is then
  2. caught by bro.js.

From there onwards, I have hard time following the events and different internal states. If you could point me to the right direction, I can try to fix it myself. This is, assuming it is actually in the responsibility of your plugin.

nikku commented 8 years ago

Could you try to build an integration test for the problem you describe and provide that as a PR? Doing that is pretty straight forward.

With that in place we can give you further assistance on what may go awry.

bengro commented 8 years ago

of course, sounds like a plan. Let me come back to you.

nikku commented 8 years ago

Thanks for the PR.

I do not quite understand how there could be valid JavaScript code compiled as the result of a compilation error. I.e. you'd like the tests to execute (being compiled from TypeScript to JavaScript) when they can actually not be compiled (due to an error).

How should that work?

bengro commented 8 years ago

Not including typing files for JavaScript dependencies which live on the global scope will mean that TypeScript at compile time fails as it does not know about the existence of those libraries. When things are bundled the actual source code ends up in the bundle, making the code sound at run time. TypeScript declaration are merely a tool to provide developers with static analysis at build time. They do not and cannot infer about the soundness at runtime.

We have the case that we are monkey patching some "incompletely typed" JavaScript of angular2. Doing so we write valid JavaScript that works, but because of the lack of typing does not "compile" without warnings.

Am I making sense?

bendrucker commented 8 years ago

The problem here is the distinction between:

tsify fires error event

and

the lack of typing does not "compile" without warnings

An error is an error. What you really need here is a way to tell TypeScript that you know about those errors and they shouldn't be raised. There's nothing preventing you from easily implementing a wrapper to do this.

I don't really know much about the TS compiler but given the info in this thread I'm 👎 on karma-browserify changing to accommodate this scenario.

bengro commented 8 years ago

What you really need here is a way to tell TypeScript that you know about those errors and they shouldn't be raised. There's nothing preventing you from easily implementing a wrapper to do this.

The TypeScript compile does not provide a way to ignore errors unfortunately. But could you elaborate on how to implement a wrapper?

I can see where you are coming from - I still think it is suboptimal behaviour to prevent the tests from running due to failures in the static analysis of TS code. I do not think that much would be lost by simply running them despite the compile errors; perhaps when a flag is activated.

bendrucker commented 8 years ago

The TypeScript compile does not provide a way to ignore errors unfortunately. But could you elaborate on how to implement a wrapper?

It'd be kind of a pain. You'd have to get really hacky and patch an .emit function where the error is being emitted. I don't really know enough about TypeScript to tell you.

Based on the discussion here I'm going to close this out. I know it sucks to be stuck right now but I don't see a way to generalize here.

nikku commented 8 years ago

Thanks guys for sorting that out.

@bengro Based on the discussion I feel time is better invested to request that feature in tsify or the typescript compiler.