Ref #76: Pass program for performance gain

[mtraynham]

As suggested in #76, we can pass the program so it's only created once per build. Alternatively, you could have the loader generate just once, instead of having the user pass it. But, I use both gulp-tslint and tslint-loader, so the consistency there is nice.

[sonicoder86]

@mtraynham can we restrict it only to typeCheck? I mean only use the provided program if typeCheck is true. I would stay with one switch for one feature.

[mtraynham]

Just so I understand, remove the program option and have the tslint-loader generate it (only once) instead?

Edit: Sorry, still early here. You want typeCheck to be a requirement, even if they pass program, got it.

[mtraynham]

Alrighty, rebased. When users provide typeCheck: true, they can also provide the program option, or it will be resolved for them, created only once, and cached back to the options object for reuse.

[sonicoder86]

@mtraynham seems good, added one review

[mtraynham]

Actually, my mistake @blacksonic . This is still creating the program object every file... It looks like the options object is recreated every time a new file is linted. We'll have to store the program on the webpack instance.

[mtraynham]

@blacksonic I think you may have annotated an old commit. That options object is getting recreated every time a new module runs through the loader (via these lines). We can't store the program on the options object because it get's discarded. Instead, I decided to store it on the webpackInstance.options, which is the same for the life of the process, and represents the original passed in options. It is stored as tsconfigProgram.

[tplk]

Can you merge this PR please? It really gives an enormous perfomance boost.

disabled - 9389ms
enabled && typeCheck: false - 12414ms
enabled && typeCheck: true - 75928ms

https://github.com/wbuchwalter/tslint-loader/pull/78
enabled && typeCheck: true - 14189ms

[zuzusik]

I just checked it locally - works as intended only for the first time - in watch mode shows exactly the same, "cached" errors with every incremental builds.

This should be fixed and also we should add a test to ensure it is capable to work correctly in watch mode?

[tplk]

Any idea when this is going to be resolved?

[pelotom]

nudge

This is a major inefficiency and it would be great to see this PR merged!

[mxl]

Guys, any news?

[sonicoder86]

The PR is not a full solution, because the first result gets cached

[mtraynham]

Rebased and corrected. When using Webpack watch mode, use the new TslintPlugin which clears the TypeScript program after each watch run.

var TslintPlugin = require('tslint-loader').TslintPlugin;

module.exports = {
    plugins: [
        new TslintPlugin()
    ],
    module: {
        rules: [
            {
                test: /\.ts$/,
                enforce: 'pre',
                loader: 'tslint-loader',
                options: { typeCheck: true, /* Loader options go here */ }
            }
        ]
    }
}

[JoshuaKGoldberg]

Bump @blacksonic - could you please take a look?

[mtraynham]

Just giving anyone a heads up. I wrote this pull, but I'm no longer using tslint-loader.

I have moved to fork-ts-checker-webpack-plugin. It runs TypeScript typechecking and optionally tslint in a secondary node process that does not affect the Webpack build process.

ts-loader now recommends this setup. Setting the happyPack flag (derived from also using the happypack loader) it performs transpilation only (transpileOnly), while fork-ts-checker is performing the typechecking & linting. That and the addition of cache-loader & thread-loader (which is inter-changable with happypack), it has now immensely improved performance over the awesome-typescript-loader.

Example of the suggested setup

[eddyw]

@mtraynham fork-ts-checker plugin runs async. Even with async set to false, there is no way to make the bundle not emit on error. I am using tslint-loader inside happypack with tslint-loader and it actually works faster for me than running fork-ts-checker separately. Besides that, avoid emitting on error works fine. Would you reconsider giving a try running tslint-loader inside happypack with your changes? I do believe this is a better alternative but thank you in any case.

[pelotom]

@eddyw

Even with async set to false, there is no way to make the bundle not emit on error.

Does this not work?

https://webpack.js.org/plugins/no-emit-on-errors-plugin/

[eddyw]

@pelotom no. I'm using the plugin but my bundle is still emitted.

That's the reason why I switched over to try this loader.

[Skeeterdrums]

This change would definitely be a nice-to-have. I noticed the performance difference as well and couldn't get any of the other TS Lint with webpack plugins to work the way I needed.

[eddyw]

Using TSLint Language Service in VSCode, I figure why ts-loader (or other TS loader) doesn't include the plugin in the Compiler. That would mean that, in a way, tslint would run together within the compiler instead of using a separate loader and passing all result all over again from one loader to another.

Of course, by default, TS plugins get ignored during compilation. However, there is a workaround to make them work as part of the compiler. Advantage of this? Messages of tslint are part of the compiler diagnostics and tslint is run at the same time while the file is being transpiled (and type checking as well).

The problem with ts-loader is that Type Checking doesn't work with HappyPack or thread-loader which instead increases build time. So I started to work on an experiment and finally decided to write a TS loader, with those ideas in mind, that works with HappyPack/thread-loader (including Type Checking) and runs tslint within the Compiler as a TS Language Service Plugin... https://github.com/eddyw/owlpkg-typescript-loader

It works in my projects. However, it may still need some work. Part of the ideas where also taken from this thread and others.

[Skeeterdrums]

@eddyw thanks for the info, I'll take a look at it. The primary reason I want TS Linting during compilation is for Checkstyle reporting as part of the automated build process. In most other scenarios, you're right: it doesn't make sense to couple these two things.