Speed up TypeScript projects

[deftomat]

As a lot of people is complaining about TypeScript performance in CRA, I decided to enable async mode in TypeScript checker.

These changes basically brings the JS compilation times to TS projects. So, recompilation took less than 1 second instead of 3 seconds in medium size project.

The problem with async mode is that type-errors are reported after Webpack ends up recompilation as TypeScript could be slower than Babel. PR allows to emit files compiled by Babel immediately and then wait for TS and show type errors in terminal later. Also, if there was no compilation errors and any type error occurs, we trigger a hot-reload with new errors to show error overlay in browser.

Also, I wanted to start a discussion about skipLibCheck: false option in default tsconfig.json. This makes recompilations really slow and we should consider to set it to true or at least give users a big warning to let them know that it could be really slow.

The following video is showing the updated workflow with a forced 2.5 second delay for type-check to give you an idea how it works.

I'm pretty sure that PR needs some polishing and improvements but it should works as it is. Especially a "hack" with reloading the browser after type-check looks ugly to me.

cc @brunolemos as he is an initiator of an original TypeScript PR.

Should fix #5820

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[deftomat]

😞 AppVeyor failed with:

.\src\App.js
You attempted to import ../sample which falls outside of the project src/ directory. Relative imports outside of src/ are not supported.
You can either move it inside src/, or add a symlink to it from project's node_modules/.

I'm assuming that it is not related to this PR.

[Timer]

That error actually occurs on purpose and is caught. :-)

[deftomat]

@Timer I'm not sure if I understand. AppVeyor is failing as test is NOT expecting that it should fail on purpose.

[Timer]

Hm, it should be running this code:

create-react-app/tasks/e2e-simple.sh

Lines 218 to 237 in 81c0cd4

	function verify_module_scope {
	# Create stub json file
	echo "{}" >> sample.json
	# Save App.js, we're going to modify it
	cp src/App.js src/App.js.bak
	# Add an out of scope import
	echo "import sampleJson from '../sample'" | cat - src/App.js > src/App.js.temp && mv src/App.js.temp src/App.js
	# Make sure the build fails
	yarn build; test $? -eq 1 || exit 1
	# TODO: check for error message
	rm sample.json
	# Restore App.js
	rm src/App.js
	mv src/App.js.bak src/App.js
	}

Ensuring the build fails but tests continue: yarn build; test $? -eq 1 || exit 1

Is this not working correctly?

[deftomat]

Sorry, my bad. Build failed due to 60 minutes limit.

Exit code was checked correctly:

[netlify]

Deploy preview for gallant-davinci-8f9bd9 failed.

Built with commit 75400fd

https://app.netlify.com/sites/gallant-davinci-8f9bd9/deploys/5c06a1e2e470852f41aa6c16

[deftomat]

TypeScript formatter now supports messages from TSLint.

(@ianschmitz Should make master...ianschmitz:tslint easier)

[deftomat]

skipLibCheck will be true by default. See #5959

[deftomat]

Not sure how to fix the failing checks as AppVeyor is failing due to 60 minutes limit for this repo and Netlify details just redirects to the 404.

Anyway, I think that PR is ready for a review.

[iansu]

Appveyor has been broken for a while so don't worry about that. And Netlify is just to preview docs changes.

[brunolemos]

I'm taking a short break from open source to finish my project so hopefully another person can review this

[ianschmitz]

Hey @deftomat. Thanks for the PR! It's looking really good so far. I'm hoping I'll have time to give it a spin this weekend.

[Timer]

We need to make sure this works with module.hot.

[deftomat]

Yes, definitely. I just discovered that for me, it works for 80% of times with module.hot. However, sometimes, error just won't show up. Not sure why, maybe because typecheck is now so fast in small projects. It worked flawlessly (also with react-hot-loader) when TypeScript was always slower then Babel.

@mikew I really appreciate that you are trying to fix this but could you explain to us what needs to be changed and why. Just a information that your branch has a correct behavior is not really helpful.

Anyway, I thing that we need to add more tests 🤔

[mikew]

I am using plain old module.hot. The errors do make it to the hot dev client, but it seems the "ok" message that arrives shortly before them queues a "hide the error overlay" that takes places immediately after the errors are shown. That's why it can flash or seemingly not appear at all.

What my changes do is notify the hot dev client that async type checking is happening, which tells it to not hide the error overlay after hot changes have been applied. This has a downside of not clearing the errors when everything has been fixed. So another message is sent when that happens to explicitly hide the error overlay.

Those changes are in mikew@f0e1b1b and mikew@a32a163. The rest of the commits are less important but deal with handling console output better from the two log streams.

[deftomat]

@mikew What about this 321d960 ?

Turns out that there is already the hasCompileErrors flag in place. The only change is that now we are checking if it is false when we are going to dismiss error overlay.

This actually makes sense as we are handling dev-server messages in an async way with no queue. Unfortunately, until now, this "issue" sleeping in CRA for a months didn't show up.

(I'm "blaming" you @johnnyreilly 😉 You guys just make it too fast 👍)

[Timer]

If it's really that fast now, should we add this async complexity?

[johnnyreilly]

(I'm "blaming" you @johnnyreilly 😉 You guys just make it too fast 👍)

I'll take that! 😊

[deftomat]

@Timer Well, we will never be sure that it is always that fast. As mentioned by @johnnyreilly, for him, it could still took 2 seconds in large project. I think that when we have an opportunity to be sure, that TypeScript will not slow down anyone, then I think we should do it.

Also, after 2 months, it turns out that the added complexity is just 2 IFs in WebpackDevServerUtils.js. Other changes are just improvements to existing features, like better TypeScript message formatter and making sure that errors are not accidentally dismissed.

[Timer]

So does it work "perfectly now", or is there still an edge case? If this is ready, I can give it a test.

[deftomat]

I think that it finally works as expected.

[mikew]

If it's really that fast now, should we add this async complexity?

As @deftomat mentioned, it's not a lot of complexity. And it opens the door for these async errors / warnings for other plugins, which might be a win down the line. But yeah, the real gains are from using the incremental API, which works without async.

So does it work "perfectly now", or is there still an edge case?

It's working as expected for me.

[johnnyreilly]

I'd ❤️ this to be merged....

[ianschmitz]

I'll give it another spin tomorrow night, and barring any glaring issues we will get it in. Thanks for your patience!

[ianschmitz]

Thanks everyone for your help! I gave it a spin with a couple of my projects and it seems to be working really nice. TypeScript is now super fast!

I'll see if we can cut a release in the next day or two. 🎉

[ianschmitz]

v2.1.4 is now out!

[johnnyreilly]

Heya,

I can see that #6395 has been merged but it also says here that #5903 was reverted.

Could you clarify where are things now please? I'm keen to get the improved TypeScript experience out there in a way that doesn't impede people who don't want to use TypeScript...

[ianschmitz]

Hey @johnnyreilly. Yes that's correct we've temporarily reverted this PR. I mistakenly published react-dev-utils as a patch level increase when it included the breaking change to https://github.com/facebook/create-react-app/pull/5903/files#diff-6736a5f1ee20761aaa6d0f4a09249e30, as well as a missing dependency of fork-ts-checker-webpack-plugin in the package.json of react-dev-utils. This caused issues for other packages that rely on the public API of react-dev-utils.

The best short term solution was to publish a newer patch version to revert these changes so downstream consumers of react-dev-utils aren't in a broken state. We are very keen to get some form of this back into master, but just want to be careful of the breaking changes in the next revision.

[johnnyreilly]

Great - thanks for clarifying!

[timothyallan]

My day today: Typescript compiling slooooow..... Typescript compiling fast!!!.... Typescript compiling slooooow.

[jakobthomasson-zz]

So when is this PR gonna be merged into master again :(

[deftomat]

@jakobthomasson See #6406 for any progress