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

Minor performance improvements #7587

Merged
merged 2 commits into from
Sep 11, 2019
Merged

Minor performance improvements #7587

merged 2 commits into from
Sep 11, 2019

Conversation

deftomat
Copy link
Contributor

Enable ESLint cache:
We discovered that enabling the ESLint cache can reduce the time for full rebuild. In our project, the time for a warm start decreased from 20 seconds to 17 seconds.

Disable syntactic errors check:
According to documentation, fork-ts-checker can check for syntactic errors. This is basically useless for us as Babel already report those errors.

Remove useless watch option:
According to documentation, fork-ts-checker ignores the watch option when useTypescriptIncrementalApi: true.

- use ESLint cache
- disable syntactic errors check in fork-ts-checker
- remove useless `watch` option in fork-ts-checker
@eladmotola
Copy link

quick question. 2/3 of your points its about typescript users. Are those changes can affect users that dont use typescript? maybe instead of delete or change those options use the useTypeScript flag?

@deftomat
Copy link
Contributor Author

@eladmotola Well, those 2 options are already in useTypeScript condition as they are an options of fork-ts-checker which we create only when we need to support TypeScript.

@eladmotola
Copy link

@deftomat a note to myself - check the hole file first... thank you for the answer

@deftomat
Copy link
Contributor Author

Looks like automated tests are broken as errors are not related to these changes 😞

@ianschmitz ianschmitz self-assigned this Aug 28, 2019
@ianschmitz
Copy link
Contributor

Thanks for the PR @deftomat. It looks like all checkSyntacticErrors does is control whether certain messages are forwarded back to webpack from TSC: https://github.com/TypeStrong/fork-ts-checker-webpack-plugin/blob/c06ca75a7dec776881b70fe538a9368f76bfe33e/src/CompilerHost.ts#L84-L94. I don't think disabling it will net us any perf benefits is my guess.

@deftomat
Copy link
Contributor Author

deftomat commented Sep 9, 2019

@ianschmitz Good catch. But we still don't need it as Babel will catch these issues anyway. But it is up to you, should I revert that flag?

Anyway, the only noticeable performance benefit is ESLint cache.

@ianschmitz ianschmitz added this to the 3.1.2 milestone Sep 11, 2019
@ianschmitz ianschmitz merged commit 71c4d11 into facebook:master Sep 11, 2019
@ianschmitz
Copy link
Contributor

Thanks! I reverted the checkSyntacticErrors change but otherwise looks good.

@lock lock bot locked and limited conversation to collaborators Sep 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants