Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suport for noEmitOnError? #57

Closed
knaos opened this issue Oct 9, 2017 · 20 comments
Closed

Suport for noEmitOnError? #57

knaos opened this issue Oct 9, 2017 · 20 comments

Comments

@knaos
Copy link

knaos commented Oct 9, 2017

Hey guys, is it possible to use the noEmitOnError from tsconfig.json?

@piotr-oles
Copy link
Collaborator

piotr-oles commented Oct 10, 2017

Hi! :)
This plugin only checks typing errors and reports errors. There is no emit at all. I think this issue should be opened in ts-loader repository :)

@johnnyreilly
Copy link
Member

Although I haven't checked recently, I believe ts-loader already supports this (I think some of our execution tests depend on it).

@johnnyreilly
Copy link
Member

I'd close the issue if there's no more to do

@knaos
Copy link
Author

knaos commented Oct 11, 2017

Yes, thank you. @johnnyreilly ts-loader has it working. But I want to use it with happypack, and then the emit prevention won't work.

@piotr-oles
Copy link
Collaborator

piotr-oles commented Oct 29, 2017

@knaos I just realized what you want to achieve and why it won't work. If you are using transpileOnly mode, ts-loader doesn't know about semantic errors so it's not possible to block emit. The information about errors is in the fork-ts-checker-webpack-plugin.

Support for this setting would be possible only in the "async: false" mode, where type-checking and emitting is synchronized. There is fork-ts-checker-emit webpack's hook - ts-loader could listen on that hook and block emit if there would be errors.

@johnnyreilly what do you think about that?

@johnnyreilly
Copy link
Member

Yup - I'd be open to a PR on that 👍

@eddyw
Copy link

eddyw commented Nov 23, 2017

@johnnyreilly I'm curious about the status of this issue. Is it now possible to do so?

@johnnyreilly
Copy link
Member

Uh I don't think anything has been implemented related to this so I guess not.. don't actually know

@eddyw
Copy link

eddyw commented Nov 23, 2017

@johnnyreilly I'll try to implement it on ts-loader and open a PR. It will take me some time, because I'm actually hating the hacky way I'm dealing with this right now.

Actually, I was thinking about building a new loader to have accept a function in options to listen to some hooks and decide if it should allow emitting files or not. Kind of:

{ loader: 'some-loader', options: { listen: (done, fail) => { /* decide here if it should allow emitting or not */ } } }

So it could work as a general solution rather than a solution only for ts-loader. Do you know if something like this exists? If so, I won't need to code it haha

@johnnyreilly
Copy link
Member

I don't know - now's your chance!

@eddyw
Copy link

eddyw commented Nov 23, 2017

@johnnyreilly got it working!. Listening to fork-ts-checker-receive hook instead before fork-ts-checker-emit is reached, so it's possible to tell webpack not to emit on error. I will share it tomorrow, now the code is a mess.

@johnnyreilly
Copy link
Member

Great!

@eddyw
Copy link

eddyw commented Nov 25, 2017

@johnnyreilly I wrote what I was using as a webpack loader. It's a bit hacky but works:
https://github.com/eddyw/forktschecker-hook-loader

Maybe something like it could be implemented inside fork-ts-checker-webpack-plugin. I mean, if this package could ship also a loader.

Anyway, it works with my current configuration in build and watch mode. The first build is usually slower in --watch, then it's cached and every other rebuild after it is faster. Also, I actually get better results with async: false (threads: 2, happypack threads: 6)

@eddyw
Copy link

eddyw commented Dec 8, 2017

It's been some time. I figured how to make the compiler work with HappyPack or thread-loader without killing Type Checking and also allowing to noEmitOnError..
https://github.com/eddyw/owlpkg-typescript-loader

I wrote the loader from scratch. First as part of an experiment, then it turned out to work fairly well. tslint is also able to run within the loader with HappyPack, ... and cache

@vepanimas
Copy link

If that line will be invoked before shouldEmit webpack hook, we'll got it working. That's how the default NoEmitOnErrorsPlugin works, it checks compilation.errors on shouldEmit and returns false if they exist.

So if we could move this code to an earlier stage (ex. afterCompile or even shouldEmit), it would possibly fix that issue. @johnnyreilly

@Fatme
Copy link
Contributor

Fatme commented Sep 25, 2019

Hey @piotr-oles, @eddyw, @johnnyreilly, @knaos, @vepanimas,

We've hit the same issue and spent some time to investigate it.

@vepanimas,
You're absolutely correct that we'll got it working if we move the logic from emit hook to an earlier stage of webpack compilation. According to the source code in webpack repo, we need to push errors in compilation object before or at the latest on shouldEmit. This way noEmitOnErrors property in webpack.config will work out of the box.

It turns out that shouldEmit is not suitable for our case as it is SyncBailHook but we need AsyncSeriesHook. However, afterCompile seems the perfect solution as it is async hook and is executed before shouldEmit.

@johnnyreilly
Copy link
Member

Do you want to raise a speculative PR and let's see where it goes?

@Fatme
Copy link
Contributor

Fatme commented Sep 25, 2019

@johnnyreilly,

Here is the PR - #337.

@Fatme
Copy link
Contributor

Fatme commented Sep 30, 2019

@johnnyreilly,

Did you have the chance to take a look at the PR?

@johnnyreilly
Copy link
Member

Sorry I'm currently snowed @Fatme

I will try and take a look but it won't be immediately I'm afraid

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants