search

TypeStrong / fork-ts-checker-webpack-plugin

Webpack plugin that runs typescript type checker on a separate process.
MIT License
1.94k stars 240 forks source link

Extremely high compilation time for production build on v5 #453

Closed IlCallo closed 2 years ago

IlCallo commented 4 years ago

Current behavior

We are about to release Quasar Framework CLI v2 (@quasar/app), and really want to integrate fork-ts-checker new version. We are testing it on real world (private) projects and I noticed an extreme (talking about some minutes, 18s vs 183s) slowdown of production compilation in a particular scenario, for which I cannot pin-point the cause.

I could use some guidance, because the behaviour I'm experiencing is pretty random and don't know how do debug it further. Right now my instinct tells me there is something wrong with our configuration of v5 and the tsconfig.json file, probably because we add and configure the plugin into @quasar/app and not into a webpack config file into the project root.

~Or some kind of incompatibility with @typescript-eslint pre v3~ EDIT: nope, upgraded to v3 and problem persist. Also, clean test project didn't had problems with pre v3

Enabling typescript.profile using the only configuration which avoids the slowdown (see below) produce no output. Enabling typescript.profile when the problem is present I got this

Parse Configuration:          0.03 s
Create Watch Compiler Host:   0.00 s
I/O Read:                     0.13 s
Parse:                        0.93 s
ResolveTypeReference:         0.04 s
ResolveModule:                0.90 s
Program:                      2.12 s
Bind:                         0.84 s
Check:                        0.90 s
transformTime:              143.05 s

Reverting to v4.1.6 fixes the problem, so I'm fairly sure the problem is in the new major version (or our configuration).

This is how we internally enable TS support into the Quasar webpack chain: https://github.com/quasarframework/quasar/blob/9133883bb3f2a3764b697bad8ffc5e313502c4ea/app/lib/webpack/create-chain.js#L154-L182

Notice we always include vue ts support into the options via webpack chain as

typescript: {
  extensions: {
    vue: true
  }
}

In Quasar configuration we allow the user to customize your plugin options like

supportTS: {
  tsCheckerConfig: {
     // ts-checker options
  }
},

Into our starter kit we automatically setup the configuration depending on initial user choices: https://github.com/quasarframework/quasar-starter-kit/blob/19bd2ec30086d1086b7e44929bed71273d3d41a4/template/quasar.conf.js#L26-L33

Experiments till now

supportTS: true

With no eslint configuration, the problem persist

supportTS: {
  tsCheckerConfig: {
     eslint: {
          enabled: true,
          files: ['./src/**/*.ts', './src/**/*.js', './src/**/*.vue']
        }
  }
},

With eslint enabled and this eslint.files glob patterns the problem is solved but I get the error below error, no lint error is displayed for Vue and TS files and typescript.profile output is not shown. All this stuff makes me think nothing is actually being processed and that's why there's no slowdown.

Error: No files matching '/home/----/src/**/*.js' were found.
Error: No files matching '/home/----/src/**/*.js' were found.
    at FileEnumerator.iterateFiles (/home/----/node_modules/eslint/lib/cli-engine/file-enumerator.js:272:27)
    at iterateFiles.next (<anonymous>)
    at CLIEngine.executeOnFiles (/home/----/node_modules/eslint/lib/cli-engine/cli-engine.js:761:48)
    at /home/----/node_modules/fork-ts-checker-webpack-plugin/lib/eslint-reporter/reporter/EsLintReporter.js:36:41
    at Generator.next (<anonymous>)
    at /home/----/node_modules/fork-ts-checker-webpack-plugin/lib/eslint-reporter/reporter/EsLintReporter.js:8:71
supportTS: {
  tsCheckerConfig: {
     eslint: {
          enabled: true,
          files: './src/**/*'
        }
  }
},

With eslint enabled and this eslint.files glob patterns the problem persist and I get an error for any extension @typescript-eslint cannot process and for all Vue files. This last bit is strange, because Vue files are marked to be processed. One more point on the "there's something strange with tsconfig.json" path.

Ex.

src/assets/icons/my-icon.svg
[unknown]: Parsing error: "parserOptions.project" has been set for @typescript-eslint/parser.
The file does not match your project config: src/assets/icons/my-icon.svg.
The extension for the file (.svg) is non-standard. It should be added to your existing "parserOptions.extraFileExtensions".
src/components/_fullscreen-dialog.scss
[unknown]: Parsing error: "parserOptions.project" has been set for @typescript-eslint/parser.
The file does not match your project config: src/components/_fullscreen-dialog.scss.
The extension for the file (.scss) is non-standard. It should be added to your existing "parserOptions.extraFileExtensions".
src/components/my-component.vue
[unknown]: Parsing error: "parserOptions.project" has been set for @typescript-eslint/parser.
The file does not match your project config: src/components/my-component.vue.
The file must be included in at least one of the projects provided.
supportTS: {
  tsCheckerConfig: {
     eslint: {
          enabled: true,
          files: ['./src/**/*.ts', './src/**/*.vue']
        }
  }
},

With eslint enabled and this eslint.files glob patterns the problem persist.

Miscellaneous info

The project tsconfig is in the project root folder and extends the preset provided by Quasar (https://github.com/quasarframework/quasar/blob/dev/app/tsconfig-preset.json)

{
  "extends": "@quasar/app/tsconfig-preset",
  "compilerOptions": {
    "baseUrl": "."
  }
}

I'm using Vue Composition API package (v0.5 right now), but the clean Quasar project I created uses it too and doesn't seems to be affected.

Expected behavior

Avoid slowdown.

Steps to reproduce the issue

I'm not able to replicate the problem with a fresh Quasar project using latest unreleased packages and starter kit.

Environment

piotr-oles commented 4 years ago

Thank you for reporting this issue. I hope this explanation will help you with debugging:

Starting from version 5, eslint has its own process, separate from typescript checker. It means that eslint configuration should not affect typescript check time (except IO as the file system is shared) and vice versa. It can slow down the time to show issues as it uses Promise.all but not the check itself.

Currently, we don't have implemented profiling for vue extension and for eslint. I can add it - maybe it would help you to find the issue.

Also, I think there is lack of documentation on how to set up eslint for .vue files. By default, the plugin sets:

  options: {
      extensions: ['.ts', '.tsx', '.js', '.jsx'],
  }

which doesn't include .vue files. You can overwrite this by passing it as eslint.options.extensions

IlCallo commented 4 years ago

Consider that the problem isn't probably related to ESLint. As I specified, the problem comes up even when linting isn't even enabled.

That let apart, I think I tried the linter with Vue files and I seem to recall that it worked anyway (probably because it got up the eslint configuration).

After a lot of trial and errors, I noticed that removing some ts aliases solved the issues. In particular components, pages and layouts.

Gonna study a bit what's happening there and what do they have in common

EDIT: tried to add eslint options as you said, no effect ~EDIT2: can confirm Vue linting isn't actually working, even if I add .vue extension into the options as you proposed~ EDIT3: note that all 3 those aliases usually references Vue files, there could be some problems in that area EDIT4: forget about EDIT3, Vue linting is working ok (even without the additional options you wrote before)

piotr-oles commented 4 years ago

Cool, thanks for digging into this 👏 I would like to help, but it's hard without a reproduction repository.

You can clone the plugin repository and add performance.markStart and performance.markEnd in the vue extension to check if it's a bottleneck. The example usage is in the TypeScriptReporter. Then you can build it (yarn build) and link it to your project (using yarn link).

I have to admit that we don't have end-to-end tests for .vue linting. The assumption is that eslint linter should be capable of doing everything that eslint command is capable of. I'm not a Vue.js developer, so I'm not familiar with the eslint support for .vue files. Is it something that eslint command handles well, but the plugin doesn't?

IlCallo commented 4 years ago

I added a new edit on previous message: seems like I was doing something wrong in tsconfig, Vue linting works as expected even without the extension options you mentioned.

Right now, I managed to make my project work (and it's blazing fast!) with this configuration:

{
  "extends": "@quasar/app/tsconfig-preset",
  "compilerOptions": {
    "baseUrl": ".",
    "paths": {
      "src/*": ["src/*"],
      "app/*": ["*"],
      // "components/*": ["src/components/*"],
      // "layouts/*": ["src/layouts/*"],
      // "pages/*": ["src/pages/*"],
      "assets/*": ["src/assets/*"],
      "boot/*": ["src/boot/*"]
    }
  }
}

We're gonna release q/app v2 (which will support this package both v4 and v5) and try to gather more examples on the wild to pinpoint the issue. Also, it will be easier for us to provide some repro when v2 is officially out.

Gonna try what you suggested next week.

If this could ring some bell to you, our current investigative trail (lol) brought us to vue-router files and it's dynamic import syntax for layouts and pages. Components could be affected indirectly because loaded by layouts an pages? Maybe it's something not working related to dynamic imports and dependencies of files retrieved in that way?

Example:

const routes: RouteConfig[] = [
  {
    path: '/',
    redirect: () => ({ name: isAuthenticated() ? 'home' : 'login' })
  },
  {
    path: '/',
    component: () => import('layouts/guest.vue'),
    beforeEnter: redirectIfAuthenticated,
    children: [
      {
        path: 'login',
        name: 'login',
        component: () => import('pages/login.vue')
      },
      {
        path: 'forgot-credentials',
        name: 'forgot-credentials',
        component: () => import('pages/forgot-credentials.vue')
      }
    ]
  },
  {
    path: '/',
    component: () => import('layouts/authenticated.vue'),
    beforeEnter: redirectIfGuest,
    children: [
      { path: 'home', name: 'home', component: () => import('pages/home.vue') },
      {
        path: 'statistics',
        name: 'statistics',
        component: () => import('pages/statistics.vue')
      },
      {
        path: 'registry',
        name: 'registry',
        component: () => import('pages/registry.vue')
      },
      {
        path: 'administration',
        name: 'administration',
        component: () => import('pages/administration.vue'),
        beforeEnter(to, from, next) {
          if (!isAdmin()) {
            next(false);
          } else {
            next();
          }
        }
      }
    ]
  }
];
piotr-oles commented 4 years ago

Hmm... Hard to tell if dynamic imports can be an issue. We use standard TypeScript API, with a custom fileExists and readFile functions which interpret src/some-file.vue.ts as src/some-file.vue (because TypeScript doesn't support custom extensions and import 'src/some-file.vue'; can be resolved only to src/some-file.vue.ts, src/some-file.vue.tsx, or src/some-file.vue.js). I've opened https://github.com/microsoft/TypeScript/issues/38736 proposal to allow compiler plugins for better Vue.js support, so you can give a 👍 there to let know TypeScript team that this is a desired feature. So we don't hack around import statements.

"app/*": ["*"], looks suspicious for me - maybe it makes other aliases hard to resolve in the TypeScript?

IlCallo commented 4 years ago

Liked the proposal :+1: Will let you know next week if I discover something more.

I don't think it's about "app/*": ["*"], the problem was still there when I disabled everything except the 3 paths I mentioned yesterday. Also, it's a pretty popular path alias, even if you usually find it as ~/* or @/* instead

jods4 commented 4 years ago

@piotr-oles

Also, I think there is lack of documentation on how to set up eslint for .vue files. By default, the plugin sets:

  options: {
      extensions: ['.ts', '.tsx', '.js', '.jsx'],
  }

which doesn't include .vue files. You can overwrite this by passing it as eslint.options.extensions

I'm hijacking this issue but wanted to say: this ☝️ Just randomly noticed my vue files aren't linted and was trying to figure out why. Glad I found this. It would be nice to add a note in the Vue section of the readme, or even better, add .vue automatically to the default extensions when Vue support is enabled.

johnnyreilly commented 4 years ago

It would be nice to add a note in the Vue section of the readme,

Would you like to submit a PR?

IlCallo commented 4 years ago

Finally got little time to come back at this. I see you added some items to the time stats, here what do I get now with dev mode

Parse Configuration:                  0.10 s
Create Watch Compiler Host:           0.00 s
I/O Read:                             0.15 s
Parse:                                2.98 s
ResolveTypeReference:                 0.07 s
ResolveModule:                        1.35 s
Program:                              4.77 s
Bind:                                 3.77 s
Check:                               10.06 s
transformTime:                      150.53 s
Emit:                               153.74 s
Semantic Diagnostics:               164.09 s
Create Watch Program:               172.81 s
Poll And Invoke Created Or Deleted:   0.07 s
Queued Tasks:                         0.00 s

and build mode

Parse Configuration:                  0.07 s
Create Watch Compiler Host:           0.00 s
I/O Read:                             0.24 s
Parse:                                2.68 s
ResolveTypeReference:                 0.08 s
ResolveModule:                        1.45 s
Program:                              4.66 s
Bind:                                 2.84 s
Check:                                7.49 s
transformTime:                      139.46 s
Emit:                               142.29 s

Will continue my debugging next week and let you know if I discover something new

IlCallo commented 4 years ago

Some comments on DX:

I tried to add a Vue-related performance measurements, but seems like I'm missing something obvious:

Meanwhile I enabled all diagnostic options like this

        typescript: {
          extensions: {
            vue: true
          },
          diagnosticOptions: {
            syntactic: true,
            semantic: true,
            declaration: true,
            global: true
          },
          profile: true
        },
        eslint: {
          files: './**/*.{ts,js,vue}'
        },
        logger: {
          infrastructure: 'console'
        }

and got

Parse Configuration:                  0.06 s
Create Watch Compiler Host:           0.00 s
I/O Read:                             0.14 s
Parse:                                2.49 s
ResolveTypeReference:                 0.06 s
ResolveModule:                        1.14 s
Program:                              4.02 s
Bind:                                 2.67 s
Syntactic Diagnostics:                0.23 s
Global Diagnostics:                   0.01 s
Check:                                7.81 s
transformTime:                      307.87 s
Emit:                               155.59 s
Semantic Diagnostics:               163.66 s

I cannot find some marks name in the codebase (transformTime, Emit, etc) so I guess those comes directly from TS via getDiagnosticsOfBuilderProgram method calls. Notice that Declaration Diagnostics isn't present even if should. Dunno what does this mean.

Any idea on how to proceed further?

IlCallo commented 4 years ago

I'm exploring TS perf issues world, seems there are quite a bit slowdown problems around. First two I found:

It may be something related to Quasar types? Does this ring any bell? Dunno why the problem appears only on v5 and not v4 tho

Running ./node_modules/.bin/tsc --noEmit false --declaration --emitDeclarationOnly --extendedDiagnostics finish in 13s only, so I'm pretty sure the problem is in Vue files (which are checked only after webpack transformation)

johnnyreilly commented 4 years ago

Fascinating - I hadn't heard of "Quasar types"

IlCallo commented 4 years ago

I mean, Quasar Framework typings:

(Or was it some pun I didn't get :thinking: )

IlCallo commented 4 years ago

More specific question: any idea on how I can start profiling TS stuff while is being processed by the plugin (or before, or after, anything), both in v4 and v5?

johnnyreilly commented 4 years ago

Although it's primitive - simply hacking in console.log statements will get you a long way. It's surprisingly effective even if you feel that "there must be a better way"

bodinsamuel commented 4 years ago

Hi team, +1 on the compilation time (but there is so much involved I'm not sure where it comes from). Once thing I share with the initial issue is that it reports error for .scss which really should not be parsed by eslint as the default extensions suggest. I suspect some glob is too greedy.

  new ForkTsCheckerWebpackPlugin({
        typescript: {
          enabled: true,
          configFile: 'tsconfig.json',
          build: true,
          mode: 'readonly',
          diagnosticsOptions: false,
          compilerOptions: {
            skipLibCheck: true,
          }
        },
        eslint: {
          enabled: production ? false : true,
          files: './src/**/*',
        },
      })
[...100s of...]
ERROR in src/views/[...]/index.scss
[unknown]: Parsing error: "parserOptions.project" has been set for @typescript-eslint/parser.
The file does not match your project config: src/views/[...]/index.scss.
The extension for the file (.scss) is non-standard. You should add "parserOptions.extraFileExtensions" to your config.
IlCallo commented 4 years ago

On Quasar framework part, we decided to wait a bit to see if someone else bump into this, as I'm just running in circles when trying to debug this problem and we must focus on some other features. Will come back to this problem in some months and start debugging again!

nfour commented 3 years ago

Just wanted to chime in and bump this.

This plugin seemed fine on 5.2.0 for a while till my own project grew, and then some kind of threshold occurred and it exploded in compile time.

Seems related to MobxStateTree & MaterialUI, which are known for their complex types.

Edit:

tsc builds in under 12s, while the plugin takes ~30minutes now (120% cpu).

tsc --watch also hangs for me, so this could be an upstream issue https://github.com/microsoft/TypeScript/issues/25023

Seems related to MobxStateTree & MaterialUI, which are known for their complex types.

Would be ideal to see if TypeScript@4.1.0's extended perf profiling features can be integrated here to map long

piotr-oles commented 3 years ago

We use the same API as the tsc --watch so it seems to be a source of the problem

nfour commented 3 years ago

We use the same API as the tsc --watch so it seems to be a source of the problem

@piotr-oles have you looked into supporting https://github.com/microsoft/TypeScript/issues/40124 4.1.0's new profiling outputs?

More info here https://github.com/microsoft/TypeScript/pull/40063

A projects file paths could be mapped to the types.json/trace.json output, then rendered as a ordered list or tree based on costs.

Might even make sense to emit a warning based on a threshold for type resolution time to let the user know when a new dependency or interface has a large impact.

piotr-oles commented 3 years ago

@nfour I've added a support for "generateTrace" in 6.0.0-alpha.2 :)

IlCallo commented 3 years ago

Good! We're working on Quasar v2 (with Vue3 support) which means we'll have to migrate to 5+ version of fork-ts-checker. I'll probably have to come back on this problem in next couple months and check if I can finally find the root cause, any additional tracing tool could be useful!

nfour commented 3 years ago

@nfour I've added a support for "generateTrace" in 6.0.0-alpha.2 :)

https://github.com/nfour/ts-rank whipped this up to quickly parse generateTrace and show some useful numbers. Still early tho but I've used it to fix some perf issues already.

kelunik commented 3 years ago

For my setup this slow down seems to mainly come from eslint, I guess it doesn't work fine with webpack 5's cache.

Initial build Second build
Entire plugin disabled 54689 ms 6217 ms
eslint.enabled: false 54045 ms 16519 ms
eslint.enabled: true 114185 ms 112230 ms
piotr-oles commented 3 years ago

@IlCallo The newest TypeScript Beta version contains a fix that could potentially address some of these performance issues. Could you test it?

IlCallo commented 3 years ago

Hey there, you mean TS 4.3 beta? What fix in particular should be useful in our case?

Also, with are in the verge of releasing Quasar v2 (and @quasar/app v3) based on Vue3 and webpack 5, we updated this package dependency to v6 directly. The project I used for this issue reproduction unluckily still have some blockers while migrating to Quasar v2 so I cannot test it right now. Since we released Quasar v2 beta, only a person reported this same problem, but he too could not pin-point the problem.

Will try to finish my project migration to Quasar v2 and try it out again

piotr-oles commented 3 years ago

The fix: https://github.com/microsoft/TypeScript/pull/43314 :)

uccmen commented 3 years ago

@piotr-oles @johnnyreilly , is the latest version of fork-ts-checker-webpack-plugin compatible with webpack 5 ?

piotr-oles commented 3 years ago

yes, we have e2e tests that uses webpack 5

caiquelsousa commented 3 years ago

I believe that this is related https://github.com/TypeStrong/fork-ts-checker-webpack-plugin/issues/612

IlCallo commented 2 years ago

I cannot reproduce anymore the problem after migrating to Quasar v2, which uses Webpack5 and latest version of this package Closing this since my original problem seems to be solved