-
Notifications
You must be signed in to change notification settings - Fork 65
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
typeCheck is too slow #76
Comments
Is it realistic to have more than one loader linting inside a Webpack configuration? |
Are you suggesting just caching the You can easily have multiple webpack configurations running at the same time, with different lint options etc. |
Maybe caching based on the webpack instance is ok. |
If the webpack instance is recreated per webpack config that should work... |
We're seeing this issue too. Simply enabling the Using tslint-loader 3.5.3, tslint 5.2.0, webpack 2.5.1. |
same here |
1 similar comment
same here |
We also have performance issues with tslint-loader. Linting is much slower compared to the tslint CLI, even without the Now I wanted to create my own tslint watcher and so I was playing around with the tslint API and chokidar. I found out during my testing that it's not a good idea to cache the I never used |
For reference, this is the example code I tested this with: const linterConfigFile = path.resolve(watchDir, "tslint.json");
const tsConfigFile = path.resolve(watchDir, "tsconfig.json");
// Tried to store program instance here, but linter gives wrong results then.
// const tsProgram = Linter.createProgram(tsConfigFile);
const lintFile = (filePath) => {
// necessary to create tsProgram for every lint...
const tsProgram = Linter.createProgram(tsConfigFile);
const linter = new Linter({ formatter: "codeFrame" }, tsProgram);
const config = Configuration.findConfiguration(linterConfigFile, filePath).results;
const fileContent = tsProgram.getSourceFile(filePath).getFullText();
linter.lint(filePath, fileContent, config);
console.log(linter.getResult().output);
};
chokidar.watch(watchDir, chokidarOptions)
.on("change", (filePath) => {
console.log("changed:", filePath);
lintFile(path.resolve(watchDir, filePath));
})
.on("error", console.error.bind(console)); |
@simonvizzini Can you confirm it is also on the pull request to address this issue? |
Ah, I'm sorry, I totally missed the pull request. This is strange, I'm sure @mtraynham has tested this, and I guess it worked for him. And if it works in I'm at home now and I won't be able to investigate this further until monday, but I'll do a quick test later today on my linux machine at home. |
I haven't had much time to investigate this further but I found this interesting note in the gulp-tslint README regarding the typeCheck feature.
This is exactly what I'm experiencing when I cache the program instance, I get back cached results. And if I understand gulp correctly then the gulp task is executed every time for every file change in watch mode, and thus creating a new |
@simonvizzini Yeah, that does look like a problem with caching the Program object with only the loader. Although, I assume this is only a problem because you are using Dev Server or some variant of watch mode. That PR may be ok for simple builds. Alternatively, adding a compiler plugin for this loader which supports events for The awesome-typescript-loader does something similar, so it only compiles changed files. |
confirming the slowness - build time increased from 1.5 minutes to 15 minutes |
@mtraynham you are right that this would work for a single run webpack build, but webpacks Also, during my own tslint performance testing I found out that creating the Here is a comparison between running tslint on our codebase directly and using tslint-loader: webpack without tslint-loader (includes assets, less, etc): ~30 secs Using tslint-loader with type checking takes an additional 50 seconds for our webpack build to complete. But using tslint directly from the CLI instead takes only around 6 seconds. So the performance issues people are reporting here aren't really tslint or typechecking related. As I see it the problem must be either in tslint-loader or webpack itself. I wrote my own tslint-watcher based on the example I posted in a comment above. It works fine and is super fast. Though it would be best to look into tslint-loader/webpack and try to fix any problems there, I just don't have enough time to do that. |
Looks we need to teach tslint-loader 2 things:
|
I can confirm this issue here as well. |
So any news on this? |
It's basically unusable. Build time increases by a factor of 10. Sometimes it crashes as well. |
no news regard this? any plans to fix this? |
This is not in any way a fix, but I found I can get by for now with following:
Obviously, would love to see this issue get resolved. |
I switched on https://www.npmjs.com/package/tslint-webpack-plugin It much better than this loader. |
@sanex3339 |
It's probably just not explicitly documented in the plugin. See these configuration options: |
@michal-filip thanks, that does indeed work! |
I'm now happily using |
While it is a nice workaround, I think its still better to make sure this bug is fixed in this loader and the issue should also not be closed yet. |
any news here ? |
Still insanely slow |
this issue is 6 months old, any progress on this? |
There haven’t even been any commits to this repo in the last 6 months, it seems safe to say the project is unmaintained at this point. |
You can use #78, and follow #78 (comment) to fix this. Although, I wrote that PR (shameless plug), I'm not actually using this loader any more. I have switched to fork-ts-checker-webpack-plugin (see my last #78 (comment)). ts-loader has been refactored to handle thread-loader and has offloaded type checking to the fork-ts plugin, which runs in a completely separate process from Webpack and doesn't block the build. fork-ts handles both typecheck and tslint. My builds were taking ~35 seconds. With ts-loader, thread-loader and fork-ts, it takes about ~11 seconds now. |
Looks like type check is deprecated: palantir/tslint#3322 |
Yes, we need. Instead of having to use --type-check --project, now we only need to use --project. There are no other changes. This is strictly cosmetic. |
So, is there an option that I can use to pass the "project" in webpack config? Adding tsConfigFile still gets me all the warnings about type information being required. |
Hey everyone, sorry this is issue is not fixed, we seemed to be lacking contributors in this project :) If you know of another TS linter that already works better than this one, please let me know, I can mark this project as unmaintained and link to the other project. If there is no other suitable linter and some of you are willing to be maintainer of this project, then please see : #40. |
Perhaps the best way forward is to direct users of |
@spiltcoffee Sounds good. Will add a note in the README. |
The
typeCheck
option makes builds extremely slow because theProgram
is recreated for each file.gulp-tslint solves this by forcing the user to pass the
Program
in as part of the configuration. I think the same could be done here by replacing thetypeCheck
with aprogram
option? Or maybe allow passing theProgram
instead of a boolean to thetypeCheck
? Otherwise it will have to be cached somehow across files.The text was updated successfully, but these errors were encountered: