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

Provide an option to invoke TypeScript in non-watch mode even when webpack is in watch mode #572

Closed
cnshenj opened this issue Mar 4, 2021 · 16 comments

Comments

@cnshenj
Copy link

cnshenj commented Mar 4, 2021

Feature motivation

TypeScript has some strange behavior that regular build is much faster a watch-mode build (sometimes the watch-mode build is 30x slower). This slows down forked type checking significantly.

Feature description

Provide an option to always invoke TypeScript in non-watch mode. This will make type checking time much more consistent and often shorter.
For example, with my project, tsc (complete build) is consistently completed within 60 seconds. tsc -w initial compilation takes about 9 minutes. Subsequent recompilation in watch mode time ranges from 1 second to several minutes. Even worse, if I change another file while type checking is still running, the webpack bundling process will be slowed down too.

Feature implementation

watch: "auto" (follows webpack) or "off"

piotr-oles added a commit that referenced this issue Mar 4, 2021
Use simple ts.Program and ts.CompilerHost when running webpack in
non-watch mode. This can improve check time in CI for some cases.

BREAKING CHANGE: 🧨 Use ts.Program and ts.CompilerHost for single compilation by default

✅ Closes: #572
@piotr-oles
Copy link
Collaborator

I will release an alpha version with this implemented - I would like to ask you to test if this solves your issue :)

@cnshenj
Copy link
Author

cnshenj commented Mar 5, 2021

Sure, I'll be glad to try that out. Note: it is watch or incremental that causes this issue, so be sure to turn off both. In case you are interested, here is the TypeScript bug: microsoft/TypeScript#35729.

I just found out with TS 3.9.7 our project can be built in 20 seconds (regular build, no watch, no incremental)! This means even if we use fork-ts-checker-webpack-plugin to start a full type checking pass, it is very acceptable, compared to the uncertainty in watch/incremental mode.

Also, my initial proposal of the option may not be good. I bet you can come up with something better.

@piotr-oles
Copy link
Collaborator

piotr-oles commented Mar 5, 2021

The solution that I implemented works that way:

  • if you use build: true mode, then use SolutionBuilder API, regardless of webpack
  • if you run webpack in watch mode, then use WatchProgram API
  • otherwise, use Program API

For now, there is no configuration for that. If you would like to use Program API also for watch mode in webpack, I can try to implement that with configuration options (that was the first implementation of the plugin, before WatchProgram API was available)

@cnshenj
Copy link
Author

cnshenj commented Mar 5, 2021

I think the Program API is what I need, and the whole purpose of the feature request is to enforce it, even when webpack is in watch mode.

The reason:

  • --build implicitly enables incremental build, which is slow due to TS bug 35729.
  • WatchProgram API is equivalent to TS watch mode, which is slow due to same TS bug 35729.

My current workaround is removing fork-ts-checker-webpack-plugin, running tsc in a separate task using nodemon to monitor file changes. It's not as convenient as fork-ts-checker-webpack-plugin because the bundling output and type checking output are in two different window, but at least both are fast.

@piotr-oles
Copy link
Collaborator

I found very interesting PR: microsoft/TypeScript#42960. I will publish alpha version anyway, but I will wait for this PR before releasing this on the latest channel. Maybe it will solve the WatchProgram issue :)

piotr-oles added a commit that referenced this issue Mar 5, 2021
Use simple ts.Program and ts.CompilerHost when running webpack in
non-watch mode. This can improve check time in CI for some cases.

BREAKING CHANGE: 🧨 Use ts.Program and ts.CompilerHost for single compilation by default

✅ Closes: #572
@sokra
Copy link

sokra commented Mar 5, 2021

Maybe it will solve the WatchProgram issue :)

Yes that was the intent of this PR.

I figured that even this that improvement fork-ts-checker-webpack-plugin is a little bit slower compared to tsc --watch directly. Maybe that's because of the different Filesystem...

@piotr-oles
Copy link
Collaborator

I will profile the plugin again after releasing your PR :)

@cnshenj
Copy link
Author

cnshenj commented Mar 5, 2021

Maybe it will solve the WatchProgram issue :)

Yes that was the intent of this PR.

I figured that even this that improvement fork-ts-checker-webpack-plugin is a little bit slower compared to tsc --watch directly. Maybe that's because of the different Filesystem...

Do you have any idea how "slower" it would be. Currently, with my project, a non-watch build takes 20 seconds. tsc --watch takes 9 minutes! I didn't measure fork-ts-checker-webpack-plugin, but it is not only slow, it will also block webpack bundling if I make another file change while previous type checking is still in progress.

I'm hoping with a proper fix, tsc --watch will take just a little longer than non-watch build, while fork-ts-checker-webpack-plugin may add a little more. But those should be just the first pass, file changes should trigger a watch build pass that is much faster.

@piotr-oles
Copy link
Collaborator

🎉 This issue has been resolved in version 7.0.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@sokra
Copy link

sokra commented Mar 5, 2021

Maybe it will solve the WatchProgram issue :)

Yes that was the intent of this PR.

I figured that even this that improvement fork-ts-checker-webpack-plugin is a little bit slower compared to tsc --watch directly. Maybe that's because of the different Filesystem...

Do you have any idea how "slower" it would be. Currently, with my project, a non-watch build takes 20 seconds. tsc --watch takes 9 minutes! I didn't measure fork-ts-checker-webpack-plugin, but it is not only slow, it will also block webpack bundling if I make another file change while previous type checking is still in progress.

I'm hoping with a proper fix, tsc --watch will take just a little longer than non-watch build, while fork-ts-checker-webpack-plugin may add a little more. But those should be just the first pass, file changes should trigger a watch build pass that is much faster.

I was talking about 10-20%, so it may take 25-30s. But it could also benefit from incremental mode so you only have to pay that on the first compile.

It may be worth trying out my typescript PR and check if that fixes your 9min issue. Clone it, npm i, yarn gulp local, and run node .../typescript/built/local/tsc.js --watch in your project folder. Leave a comment on the PR. Maybe add "diagnostics": true to your tsconfig to get timing info.

@piotr-oles
Copy link
Collaborator

@cnshenj did you tried @sokra's PR or the alpha version of the plugin?

@cnshenj
Copy link
Author

cnshenj commented Mar 22, 2021

@piotr-oles I haven't. As we discussed before, the fix I need is to make TypeScript run in non-watch mode, even when webpack is in watch mode. But according to what you described, the alpha version will make TypeScript run in watch mode when webpack is in watch mode.

Currently I use nodemon to monitor project changes and start a full TypeScript compilation when a change is detected. A full compilation pass only needs 20 seconds, compared to several minutes by incremental compilation in watch mode.

I also had some discussion with @sokra in the PR. I believe he made changes to address the specific scenario I described (modify several files and save at the same time). Now we are just waiting for the TypeScript team to incorporate the fix and release TypeScript 4.3.

@piotr-oles
Copy link
Collaborator

That makes sense, let's wait :)

@piotr-oles
Copy link
Collaborator

@cnshenj Could you try the latest TypeScript beta version? It contains @sokra's fix :)

@piotr-oles piotr-oles added performance typescript Related to TypeScript labels Apr 21, 2021
@piotr-oles
Copy link
Collaborator

I will assume it fixed the problem. If not, I will re-open the issue :)

piotr-oles added a commit that referenced this issue Jan 29, 2022
* feat: use simple program for single compilation (no watch) (#574)

Use simple ts.Program and ts.CompilerHost when running webpack in
non-watch mode. This can improve check time in CI for some cases.

BREAKING CHANGE: 🧨 Use ts.Program and ts.CompilerHost for single compilation by default

✅ Closes: #572

* feat: remove eslint support (#607)

BREAKING CHANGE: 🧨 ESLint no longer supported by the plugin

* chore: merge main changes (#619)

* test: use karton for e2e tests (#627)

Use external package to handle e2e tests to simplify code base

* feat: drop support for webpack 4 (#638)

BREAKING CHANGE: 🧨 Webpack 4 is no longer supported. Please upgrade to Webpack ^5.11.0 or use an older version of the plugin.

* feat: improve error formatting to match webpack 4 convention (#641)

* feat: upgrade dependencies

Upgrade dependencies that doesn't require Node version bump.

* chore: remove unused dependencies

* feat: drop support for node 10

Node 10 is no longer maintained

BREAKING CHANGE: 🧨 Require Node.js >= 12

* chore: upgrade dev dependencies

* feat: remove support for TypeScript < 3.6.0 (#643)

BREAKING CHANGE: 🧨 Drop support for TypeScript < 3.6.0

* feat: port changes from main branch (#649)

This commit contains fixes from the main branch

* fix: require typescript@^3.8.0 for build: true mode (#672)

SolutionBuilder API is buggy for TypeScript < 3.8.0. To reduce maintenance burden, we bump minimal TypeScript version for { build: true } mode to 3.8.0.

BREAKING CHANGE: 🧨 Minimal version of TypeScript with { build: true } mode changed from 3.6.0 to 3.8.0

* refactor: add eslint rules to enforce import order (#671)

* feat: migrate from reporters to workers (#691)

Use simple functions and modules to simplify worker code

* refactor: rename files to match convention (#693)

I found snake-case easier to read, and given that
the project doesn't use OOP a lot, having all PascalCase
names doesn't reflect the paradigm and feels unnatural.

* feat: simplify logger options (#695)

Currently, the logger options are overcomplicated. To simplify them, we
are removing logger.infrastructure option.

BREAKING CHANGE: 🧨 Changes in options: `logger.issues` becomes `logger`, `logger.devServer`
becomes `devServer`, `logger.infrastructure` has been removed

* docs: add plugin logo (#696)

* refactor: upgrade project dependencies (#698)
@piotr-oles
Copy link
Collaborator

🎉 This issue has been resolved in version 7.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

3 participants