fix: Only build typescript files in pre-commit for typescript projects

[Lukas-Kullmann]

What:

Currently, there is a bug in the lint-staged configuration: It checks if ifTypescript is truthy. Since this is a function, it will be truthy for every project. I changed the pre-commit script to move typescript compilation from lint-staged to building the whole project during pre-commit.

Why:

This is a follow-up on #141 and testing-library/dom-testing-library#572. Currently, it tries to build dom testing library as a typescript project even though it is not one (it doesn't even have typescript as a dependency). This is wrong in my opinion since the reason for the ts build there is that they want to check the typescript definitions. This is already done by dtslint.

How:

I removed the lint-staged command for building typescript and added a typescript build to the pre-commit script.

Checklist:

• Documentation
• Tests n/a
• Ready to be merged

[codecov]

Codecov Report

Merging #142 into master will decrease coverage by 0.74%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master     #142      +/-   ##
==========================================
- Coverage   87.65%   86.90%   -0.75%     
==========================================
  Files          18       18              
  Lines         332      336       +4     
  Branches       78       80       +2     
==========================================
+ Hits          291      292       +1     
- Misses         34       36       +2     
- Partials        7        8       +1     

Impacted Files	Coverage Δ
src/scripts/pre-commit.js	76.47% <62.50%> (-15.84%)	⬇️
src/config/lintstagedrc.js	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8899fe9...2fed895. Read the comment docs.

[kentcdodds]

Whoops! Thank you for this fix! One question.

[kentcdodds]

Is there a reason for this requirement to add noEmit to the config? Could we not use the flag in the command instead?

[Lukas-Kullmann]

Unfortunately, no.
When you build a typescript project, you have two options:

1. Use a tsconfig.json file (that's what we do here)
2. Provide all compiler options as command line options

And you can't mix those two.
So running something like tsc --noEmit --build tsconfig.json will not work (error TS6369: Option '--build' must be the first command line argument.) and switching noEmit and build will also not work (error TS5072: Unknown build option '--noEmit'.).

I thought about creating a custom tsconfig file just before building the ts project. It could use the extends field to include all the configuration of the project's tsconfig. But I'm not sure if this is that reliable. For example it needs to be removed again after building the project. But what happens if the user interrupts the script during the ts build? These cases would need to be handled. Handling that would require quite some code. And I'm not sure if adding this code is really worth it. Maybe having this hint (i.e. "please add --noEmit to your tsconfig file") is enough. After all, you will want to have this option set anyways if you use babel as a typescript compiler.

[kentcdodds]

I think eventually we'll have a tsconfig in kcd-scripts that my projects will extend. But for now this is acceptable. Thanks!

[MichaelDeBoey]

Wouldn't this be fixed by just calling the ifTypescript function instead? 🤔

[kentcdodds]

actually, I think it's better to run the typescript compiler across the entire code base not just the ones that changed. So I think this is a good change.

[Lukas-Kullmann]

Hey Kent, I just wanted to ping you that I answered to your review comment. I don't know if GitHub sends you an email for that.

@MichaelDeBoey Yes, it would be fixed by calling the ifTypescript method. But as Kent said, I also think that it is better to build the whole project. It might take longer to compile, but you are sure that files that did not change are still compiling (maybe the API of one of their imports changed and the build for them fails now). But since you already liked Kent's comment, I think you agree. :)

[kentcdodds]

@all-contributors please add @Lukas-Kullmann for code and docs

[allcontributors]

@kentcdodds

I've put up a pull request to add @Lukas-Kullmann! 🎉

[kentcdodds]

🎉 This PR is included in version 6.0.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀