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

feat(ts-config-webpack-plugin): Increase incremental type checking perrformance with useTypescriptIncrementalApi #43

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jantimon
Copy link
Contributor

For details please see #39

@jantimon
Copy link
Contributor Author

@0xorial We tried to activate the useTypescriptIncrementalApi in a small change 4811b7e

However we run into the following error:

ts-error

[{"file": undefined, "location": {"character": undefined, "line": undefined}, "message": "ERROR in undefined(undefined,undefined):
    TS18003: No inputs were found in config file '/home/travis/build/namics/webpack-config-plugins/packages/ts-config-webpack-plugin/test/fixtures/simple/tsconfig.json'. Specified 'include' paths were '[\"**/*\"]' and 'exclude' paths were '[\"../../../../../**/node_modules/**/*\",\"../../../../../**/test/**/*\",\"../../../../../webpage/**\",\"../../../../../performance/src/**\"]'.", "rawMessage": "ERROR TS18003: No inputs were found in config file '/home/travis/build/namics/webpack-config-plugins/packages/ts-config-webpack-plugin/test/fixtures/simple/tsconfig.json'. Specified 'include' paths were '[\"**/*\"]' and 'exclude' paths were '[\"../../../../../**/node_modules/**/*\",\"../../../../../**/test/**/*\",\"../../../../../webpage/**\",\"../../../../../performance/src/**\"]'."}]

Could you please give us a hint were we could start debugging?

@0xorial
Copy link

0xorial commented Jan 17, 2019

As I see, before the config was loaded here. With 'useTypescriptIncrementalApi' it is loaded inside typescript code, through this function.

I would compare if there is a difference between how those 2 work...

@jantimon
Copy link
Contributor Author

jantimon commented Jan 17, 2019

Thanks @0xorial that moved me to the correct point however it is entirely unrelated to your code change.

It's just that fork-ts-checker-webpack-plugin@next reports error if it is not able to do a typecheck (e.g. because no typescript file exists)

The errror is invoked here:
https://github.com/Realytics/fork-ts-checker-webpack-plugin/blob/e66e912183d4ef5d57ed262b76ceb788ba245399/src/index.ts#L772

TS18003: No inputs were found in config file '/home/travis/build/namics/webpack-config-plugins/packages/ts-config-webpack-plugin/test/fixtures/simple/tsconfig.json'

Which is not the case for fork-ts-checker-webpack-plugin@latest.

However fork-ts-checker-webpack-plugin allows to ignore diagnostic errors 👍

@jantimon jantimon force-pushed the feature/upgrade-ts-fork-checker branch from 4811b7e to 32c98e4 Compare January 17, 2019 15:56
@johnnyreilly
Copy link
Contributor

Just to publicise what @jantimon and I discussed in Slack earlier:

I'm planning to move the require("perf_hooks") into a branch which is only called when using measureCompilationTime then fork-ts-checker-webpack-plugin will be node 6 compatible save for that feature (which given it's only really of use for benchmarking seems entirely pragmatic / reasonable.)

@jantimon jantimon force-pushed the feature/upgrade-ts-fork-checker branch from 32c98e4 to e4a6ed8 Compare January 18, 2019 10:27
@johnnyreilly
Copy link
Contributor

@johnnyreilly
Copy link
Contributor

johnnyreilly commented Jan 18, 2019

@jantimon - I'm pretty sure that with the above PR this now works with node 6 once more. Caveat - you can't build it with node 6 because of webpack 5 reasons. I don't think that's an issue though.

Would you be able to test that this works with node 6? Pretty sure it should. If so then I'll merge and push out a new version.

@johnnyreilly
Copy link
Contributor

Actually, there didn't seem any reason not to ship... So I shipped: https://github.com/Realytics/fork-ts-checker-webpack-plugin/releases/tag/v1.0.0-alpha.4

Test away!

@jantimon jantimon force-pushed the feature/upgrade-ts-fork-checker branch 5 times, most recently from 1faf6d3 to e315e97 Compare January 20, 2019 10:25
@jantimon
Copy link
Contributor Author

Node 6 support is fixed with https://github.com/Realytics/fork-ts-checker-webpack-plugin/releases/tag/v1.0.0-alpha.4 - great work @johnnyreilly

Unfortunately using useTypescriptIncrementalApi: true freezes the build:

https://travis-ci.org/namics/webpack-config-plugins/jobs/482000488

broken

The same build with useTypescriptIncrementalApi: false:
https://travis-ci.org/namics/webpack-config-plugins/jobs/482000684

works

@jantimon jantimon force-pushed the feature/upgrade-ts-fork-checker branch from fbbed44 to 149c1b4 Compare January 20, 2019 11:57
@jantimon jantimon force-pushed the feature/upgrade-ts-fork-checker branch from 149c1b4 to 23883c4 Compare January 20, 2019 11:59
@jantimon
Copy link
Contributor Author

@0xorial do you have any idea how we could debug this?

@johnnyreilly
Copy link
Contributor

So it looks like it works fine on windows and Mac but has problems on Linux with large projects

@0xorial
Copy link

0xorial commented Jan 20, 2019

@jantimon not right away, but I will have a look...

@0xorial
Copy link

0xorial commented Jan 20, 2019

One idea that I have following. In non-incremental version chokidar is used to watch files inside the worker process. In incremental version we use TypeScripts' own implementation. Perhaps there is a difference regarding how those 2 behave when getting signal to exit. In that case it could be that build is freezing because our worker process does not stop.

I am not sure if I can check that quickly though... Any thoughts?

@jantimon
Copy link
Contributor Author

Thanks for the feedback @johnnyreilly @0xorial !

I guess the reason is really the file watcher.
Is there any limit for linux which does not exist for mac?

@janbiasi janbiasi assigned janbiasi and jantimon and unassigned janbiasi May 24, 2019
@ernscht ernscht added the check label Apr 8, 2021
@ernscht ernscht added the ts-config ts-config-webpack-plugin label Apr 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
check ts-config ts-config-webpack-plugin
Development

Successfully merging this pull request may close these issues.

5 participants