-
-
Notifications
You must be signed in to change notification settings - Fork 239
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
Slow Performance on Incremental Runs (Project References) #463
Comments
Hi, |
Is there any chance of getting a reproduction repository? |
https://github.com/nathanforce/monorepo-ts-projects-webpack @piotr-oles Here's my best attempt. It is very simplistic but follows the same structure/patterns as the real repo, just with far less files. Even in this tiny example I am seeing slower numbers on the webpack side when running the two commands: webpack:
tsc:
As is expected in such a contrived reproduction, the differences here are much much smaller but maybe it can help you spot something. Sorry for the delay! Cheers |
I will try to take a look at this in a few days :) |
@piotr-oles #494 seems to have helped a bit but the typecheck times using Webpack/plugin are still much higher than using tsc: |
Cool that there is a progress - I will profile it again :) |
Thanks for looking into this. I just upgraded to this version on a branch and I believe we're experiencing the same problems. |
Hi guys! I have an application built with I'd like to debug the compilation phase, and some pointers would help me a lot. E.g. where could I insert a Thanks a lot in advance! |
We don't process file-by-file - we pass configuration and host to the TypeScript API and we use this API to check for errors. I would suggest using debugger or console.log in the node_modules/typescript/lib/typescript.js file :) |
Thanks Piotr. The problem may be around the TS code; i.e. some corner case that the TS compiler doesn't "like". However, in a
The first step is executed reasonably fast. It's the second step that seems to put TS to the knees. I will somehow extract the exact parameters which are passed to the TS API. ESLint is also a player in the game; I didn't yet understand which is its exact relation to TS. Maybe the issue it's not in the TS code actually, but rather related to ESLint. |
Just FYI: babel is not running tsc, but just stripping away type information, so when you start debugging in the typescript compiler, this step will never hit anything there. |
In the case of create-react-app, |
Thanks guys for your kind hints! It does seem to be related to tsc. Running To be honest, coming from the Java world I was amazed by the power of the types system in TS. And I was very suspicious at first, having concerns related to compiler performance. It seems that at some points the "type magic" does have a non negligible cost. |
fwiw, i did some extensive benchmarking[1] and fork-ts-checker struggles the most with reference files in project references (but not main files which import the references) |
Interesting. Because I just discovered the reference / composite structure recommended by TS as a performance improvement (+ reusability flow) mechanism. |
what do you mean exactly? that you are more productive? yes absolutely, it's TS killer feature and if you change references a lot pure tsc is the fastest followed by ts-loader |
@piotr-oles do you have already any ideas how to fix fork-ts-checker's perf issues with project references or is it something which can't be fixed because of architectural reasons? |
I don't have any ideas yet - first we need to find the root cause of this issue. I'm not sure why it's slower for references. If someone has some free time, feel free to profile it to find where is the bottleneck :) |
My (temporary ?) solution for my issue:
was to monkeypatch, within CRA, the creation of useTypescriptIncrementalApi = false; |
CRA uses an older version of the ForkTsCheckerWebpackPlugin which is no longer developed. I don't think this version supports project references at all :/ I think we should drive CRA to update to a newer version |
There is indeed facebook/create-react-app#6799 To be honest, given the time I invested in CRA for monkeypatching => personally I don't find CRA a robust enough solution. Because of our attempt to reuse a common library among several projects, we were pushed towards using some symlinking. And because of this absolutely every new feature that we added: was done with a lot of pain. Initially I saw in CRA a set of best practices; a reference. But even the Jest testing mechanism is slow and we were forced to ... more monkeypatch for mocha, which has the advantage of executing tests instantaneously. |
@piotr-oles profiles are below, interestingly there's almost no output from your profiler with reference files, so i guess fork-ts-checker is not even touching those files, hence not properly supporting project references. what should i do next? change in non-reference file:
change in reference file:
|
fyi/fwiw, IDK but this slow ref transpile times might come from tsloader and not from fork-ts-checker, I did more benchmarking and added babel to the mix as a comparison and something is wrong with tsloader ref handling |
As a profile I mean basically debugging/finding the root cause of this behaviour using whatever method you like. It will require digging into typescript's codebase which can be a time-consuming task :) |
ok got it, however is it really ts or ts-loader because what I find strange is that fork-ts is not even touching reference files (if you look at the profile data above). IDK if I can help, I guess I am as busy as you are. However let me know if I can do something, otherwise I'd move on. |
microsoft/TypeScript#40808 |
I've added support for a "generateTrace" option available in typescript 4.1.0-beta in 6.0.0-alpha.2. It should help to debug performance issues :) |
@nathanforce The latest TypeScript Beta version contains a fix that could potentially fix this issue - could you test it? |
Just faced the same issue with the project freshly migrated to ts (ejected CRA + SSR build + updated fork-ts-checker-webpack-plugin). |
It seems that the bug was in the TypeScript and as the new version of TypeScript has been released, I'm closing this issue :) |
Is it possible that this issue is back? I'm using new ForkTsCheckerWebpackPlugin({
async: isEnvDevelopment,
typescript: {
configOverwrite: {
compilerOptions: {
sourceMap: isEnvProduction ? shouldUseSourceMap : isEnvDevelopment,
skipLibCheck: true,
inlineSourceMap: false,
declarationMap: false,
noEmit: true,
incremental: true,
tsBuildInfoFile: paths.appTsBuildInfoFile,
},
},
context: paths.appPath,
diagnosticOptions: {
syntactic: true,
},
mode: 'write-references',
build: true,
profile: true,
},
issue: {
// This one is specifically to match during CI tests,
// as micromatch doesn't match
// '../cra-template-typescript/template/src/App.tsx'
// otherwise.
include: [{ file: '../**/src/**/*.{ts,tsx}' }, { file: '**/src/**/*.{ts,tsx}' }],
exclude: [
{ file: '**/src/**/__tests__/**' },
{ file: '**/src/**/?(*.){spec}.*' },
{ file: '**/src/**/?(*.){stories}.*' },
{ file: '**/src/setup-tests.*' },
],
},
logger: {
infrastructure: 'silent',
},
}), I'm using |
Current behavior
Incremental typechecks are very slow.
We have a project that leverages Project References in Typescript. As the latest release of your plugin supports references we tried to upgrade. We are seeing an unusable drop in performance when type-checking this code.
For comparison, here is the same code change ran with
tsc --build --incremental
Here is our config:
Expected behavior
Incremental typechecks should be quick.
Environment
The text was updated successfully, but these errors were encountered: