noEmit and skipLibCheck should be on by default in TS compiler options

[slikts]

This is a related issue to #352. The build script works with rollup.js, not tsc, so the noEmit option should be enabled by default. Users running npx tsc shouldn't expect to have to specify --noEmit, because using tsc is a normal part of TS development. Currently npx tsc will clutter the src directory with transpiled files, which can be difficult to clean up if there are uncommitted changes mixed in.

This is important because without tsc there is no way to do project-wide type checking. The build script will only stop at the first error. The test runner will report errors, but only for files that are tested, and it's also the wrong place for type checking (#521).

[agilgur5]

Do you mean turning it on by default in the templates? Or in the build command itself?

I think your intent here is to have type-checking separated from build (or test), which makes sense to an extent.
But changing tsconfig.compilerOptions.noEmit to true would be very unexpected. The build command should emit by default, not just report type errors by default -- that is how build tools work after all, including tsc itself.

In either case, that would be a breaking change. I think #352 makes more sense than this, but #352's change doesn't actually align with how ESLint works.
An intermediate change would be to change the templates such that the lint script runs tsc --noEmit && tsdx lint. That way a workflow isn't being forced upon users (they can change it/it would be merely a recommendation) and tsdx lint isn't different from ESLint's behaviors.
Misalignment with the underlying tools & their integrations is already an increasing issue with TSDX (#514, #498, some on testing too), and that misalignment is very confusing and unintuitive, so I don't think further misaligning by changing tsdx lint itself is a good idea.

Another option is to have another command like tsdx tsc, but I have a feeling that would cause more issues than it solves.

You can also run tsdx watch --noEmit for something similar to what you're looking for.

it's also the wrong place for type checking (#521).

Again that's an opinion... Doesn't matter how many times you repeat it, it's still an opinion. ts-jest exists and has its defaults as such for a reason.

[slikts]

tsc is primarily a type checking tool that can also transpile. I don't see how it emitting transpiled files into the source directory (not the build directory) would be useful, particularly when the actual build uses Rollup.js instead, or for what users it would be a breaking change.

I forgot to mention that besides noEmit it also requires skipLibCheck, since otherwise tsc can fail when checking dependencies in node_modules.

Currently tsdx build fails on the first error, so if you want to fix all the errors so that the build would pass, you need to run tsc. Without noEmit, it's essentially a trap, since it creates many files in the source directory, requiring something like git clean -f to resolve.

There are also type errors in the test runner, but not necessarily for all files. It's an opinion that the test runner should just run tests instead of checking types, but it's well-grounded, so I'm repeating it.

Currently, to check all the source files for type errors, it requires knowing to run tsc with one or two flags, otherwise it will create an inconvenience for the user or possibly fail with a false positive.

Notably, the create-react-app TS template has noEmit and skipLibCheck enabled.

[agilgur5]

tsc is primarily a type checking tool that can also transpile

Well considering --noEmit is a flag, I don't think the TypeScript team would agree.

I don't see how it emitting transpiled files into the source directory (not the build directory) would be useful, particularly when the actual build uses Rollup.js instead, or for what users it would be a breaking change.

Sorry maybe this wasn't clear -- tsconfig.json configures both TSDX and tsc. That's where the issue is.
That's also why I asked if you meant templates or build command itself. Just changing the templates' tsconfig.json means that tsdx build would also not emit by default.
We can override tsdx build to always emit and then set the templates to have noEmit, but that override would be breaking to users that configure noEmit for TSDX. It's probably not a huge number of people, but it is breaking.

You pointed out a separate issue here that outDir should probably be set to ./dist/ by default in the templates. Though TSDX doesn't currently handle that config option too well (there's a few issues on that and #367 that still hasn't been reviewed) so it's not really "configurable" right now.

it also requires skipLibCheck, since otherwise tsc can fail when checking dependencies in node_modules

I don't actually have experience with this flag, but afaict this skips checking all declaration files, including ones you may have created yourself. You'd want the ones you created to be checked.

Currently tsdx build fails on the first error, so if you want to fix all the errors so that the build would pass, you need to run tsc

Yea you mentioned this and I guess I never really noticed this. It sounds like it would be good if outputted all errors, so I'll look into that.

Without noEmit, it's essentially a trap, since it creates many files in the source directory, requiring something like git clean -f to resolve.

Yea, that's unexpected/bad. See outDir comment above. That would fix this issue.

Notably, the create-react-app TS template has noEmit and skipLibCheck enabled.

This is a good example, especially since TSDX has borrowed pieces of CRA before. I don't know how long it's had those options; there might be a reason why Jared didn't stick with them. I know CRA reads tsconfig.json (#489 uses similar code) but I'm not sure how much of it is overridden if any (particularly for dev vs. prod builds). That may be where the difference is, will have to look.

[slikts]

noEmit has no effect on tsdx build because tsdx uses Rollup (rollup-plugin-typescript2 just checks types and transpiles). tsdx build completes fine with noEmit set to true. outDir should not be set and insted emitting with tsc should be disabled because tsc is not used as the build tool, but Rollup is. Rollup or webpack are specialized build tools and so are preferable to tsc because of all the features and plugins they have. That's why tsc is primarily a type checker that can also transpile, but it isn't used for transpiling in more advanced contexts.

skipLibCheck being disabled makes type checking fail if there are type errors in dependencies. Having to fix dependencies in node_modules to pass type checks is almost never what the user would want, and it's also not what tsdx build currently does. tsdx build also doesn't check the ambient declarations.

Not having skipLibCheck also makes builds unnecessarily slower; just tsc --skipLibCheck on my test project takes 3s, just tsc takes 10s, and tsdx build takes 30s. This is also why type checking shouldn't be coupled to building, because it delays feedback.

[agilgur5]

noEmit has no effect on tsdx build because tsdx uses Rollup (rollup-plugin-typescript2 just checks types and transpiles). tsdx build completes fine with noEmit set to true.

I didn't know this, but rpts2 apparently forces noEmit to false. I thought turning it to true would be the equivalent of a dry-run but I guess it's just ignored.

So yea, then you're right, this wouldn't be breaking. Would you like to make a PR to add noEmit to the templates?

Would have to add:

• "noEmit": true to each template
• a comment next to this option saying something like // this is only for type-checking with tsc, tsdx build forces this to false
• possibly a typecheck script? (and related docs). would just run tsc

outDir should not be set and insted emitting with tsc should be disabled because tsc is not used as the build tool, but Rollup is.

The templates can be reconfigured as a user wants, so ideally outDir should be set as well so that if they remove noEmit or something they don't fall into the trap. Also because I'm working on making the ability to change outDir.

Rollup or webpack are specialized build tools and so are preferable to tsc because of all the features and plugins they have. That's why tsc is primarily a type checker that can also transpile, but it isn't used for transpiling in more advanced contexts.

I don't disagree that they have lots of features and that in TSDX's case, tsc would just be a type checker. But lots of projects use tsc directly to compile, and even Rollup users use tsc: ezolenko/rollup-plugin-typescript2#189 . This is off-topic in any case.

skipLibCheck being disabled makes type checking fail if there are type errors in dependencies. Having to fix dependencies in node_modules to pass type checks is almost never what the user would want

facebook/create-react-app#5738 (comment) confirms that all type declaration files get skipped if this is disabled, including internal ones. I'm not sure I agree with the position that a user would "almost never" want this. If there are type errors in your dependencies, that's a problem and you should fix that (by upgrading or overriding etc). TSDX itself has an internal declaration file. This file wouldn't be type-checked if skipLibCheck is disabled.

it's also not what tsdx build currently does. tsdx build also doesn't check the ambient declarations.

Huh, weird I just confirmed that myself. It's not mentioned in rpts2's docs, so this might be a bug actually. EDIT: Filed ezolenko/rollup-plugin-typescript2#212

Not having skipLibCheck also makes builds unnecessarily slower; just tsc --skipLibCheck on my test project takes 3s, just tsc takes 10s,

I'm not sure I would characterize this as "unnecessarily" slower, it checks things that might not be correct, including any of your own declarations.
It is a big hit, but shouldn't be big overall when watching for incremental changes during development, and for production it seems quite useful.

[slikts]

So, you're a user, you get a type error in a dependency in node_modules that you don't control, and your build fails; what are you supposed to do? There wouldn't even necessarily be a straightforward patch; it might be a conflict in complex typings, or an issue with generated files (meaning that you would have to delve into the build setup of some other project to fix it, or patch generated files), etc. Remember that this makes the build fail on top of being slower.

Moreover, the ambient declarations are typically already checked at authoring time; the reason why skipLibCheck exists is because the declarations are typically already known to be correct and don't need to be checked every time.

… shouldn't be big overall when watching for incremental changes …

Incremental builds are a specific feature in TS (the incremental compiler option), and they would require noEmit to be false, and emitDeclarationOnly with a temporary outDir (that's also in .gitignore). I didn't propose it since it's more complex (the outDir needs to be unused, for example, to avoid conflicts), but it might be a good idea; it's what I use myself.

The other kind of incremental checking is watch mode, but that's only appropriate when working on something for an extended period, and even then it should be normal to check the types for the rest of the project manually, since the editor would already check the files being edited.

Your example declaration file in tsdx has essentially nothing in there to be checked (it's actually broken, but in an inconsequential way; it's not detected because tsdx itself uses skipLibCheck). For the projects that do something advanced in ambient declarations, there are special tools like tsd or dtslint. Those are advanced use cases, however.

skipLibCheck also makes the type checking noticeably slower; it slows it down several-fold for the projects that I've timed it with.

[slikts]

Options for incremental builds:

    "incremental": true,
    "emitDeclarationOnly": true,
    "outDir": ".cache/tsbuild",
    "skipLibCheck": true,

[agilgur5]

So, you're a user, you get a type error in a dependency in node_modules that you don't control, and your build fails; what are you supposed to do?

Are you saying the correct thing to do is to... ignore it??
skipLibCheck makes for less strict typing and TSDX currently has tsconfig.json set to most strict (which I believe is optimal)
I think it makes more sense to either explicitly override the types or patch or upgrade to a newer version or file an issue or check if there is a workaround available.

Moreover, the ambient declarations are typically already checked at authoring time; the reason why skipLibCheck exists is because the declarations are typically already known to be correct and don't need to be checked every time.

The keyword here is typically. An author may also have less strict typings than you might. And they may have hand-written the declarations and didn't check them. Which, as you pointed out, is something TSDX itself is doing 😕

Your example declaration file in tsdx has essentially nothing in there to be checked

It's really easy to accidentally have a type error when hand-writing declarations. And it wouldn't be checked. This problems in the internal declaration are pretty much that exactly.

(it's actually broken, but in an inconsequential way; it's not detected because tsdx itself uses skipLibCheck)

Welp, well that's ironic. One of those things that pop up when it doesn't dogfood itself: #381

skipLibCheck also makes the type checking noticeably slower; it slows it down several-fold for the projects that I've timed it with.

Based on my reading, that's the primary reason it exists, yes. I don't think trading off strictness for a performance optimization should be the default case. You can always re-configure it, but strictness should be default.

---

and even then it should be normal to check the types for the rest of the project manually, since the editor would already check the files being edited.

Not totally sure what you're trying to say here.
My point was that the performance penalty will only happen on the first build if using any form of incremental checking, and watch mode is the default script for yarn start.

Options for incremental builds:

I'm not sure what the purpose of emitDeclarationOnly and skipLibCheck are here? They're not necessary for incremental to be used.

In any case, the requirement of setting outDir makes this difficult to have as a default since outDir will be configurable soon inside of TSDX and is already used by rpts2. Would need to have something like split dev/prod tsconfigs.

---

I'm still ok with the noEmit option being added in a PR as I said previously. The rest of this is kind of a separate issue.

[slikts]

Are you saying the correct thing to do is to... ignore it??

If the build is failing due to external typings, the main actionable fix will be to disable checking them, and it particularly makes sense since the errors may be spurious and/or have no impact (like the tsdx declaration error, or the errors that allowSyntheticDefaultImports was created to ignore). Filing an issue or looking for upgrades in many cases will amount to wishful thinking.

Declaring strictness as an end in itself is not a serious approach; it depends on the specifics.

The incremental compiler option requires emitting, and since the actual build artifacts are created by Rollup, emitDeclarationOnly is a way to cache just the declarations, and I included skipLibCheck since it's part of the difference from the current config. yarn start doesn't factor into it since it's not suitable for type checking due to being slower and incomplete. Not having to wait unnecessarily for common tasks is a significant part of a good developer experience.

I've found that I should explore alternatives to tsdx that would be guided by better experienced input.

[agilgur5]

If the build is failing due to external typings, the main actionable fix will be to disable checking them

That is one possible fix, not the only one. And it is a very overbroad one at that, as it disables checking all declaration files. And, again you can change your tsconfig.json to disable skipLibCheck if that's the solution you want to use.

Declaring strictness as an end in itself is not a serious approach; it depends on the specifics.

We're talking about defaults here. Again, you can change your tsconfig.json however you want.

The incremental compiler option requires emitting, and since the actual build artifacts are created by Rollup, emitDeclarationOnly is a way to cache just the declarations

Ok, I'm confused -- I thought you meant using incremental as a way to speed up type-checking with tsc not actual production builds.

Not having to wait unnecessarily for common tasks is a significant part of a good developer experience.

I agree that's not necessary for type-checking, which is why I've said a few times now I would support noEmit and possibly a typecheck script.

I've found that I should explore alternatives to tsdx that would be guided by better experienced input.

Ok sure. Again, you can reconfigure TSDX however you want. The defaults don't seem to align with your opinions, but those are, again, opinions, and again, at least some of these don't seem to be widely supported by other users (skipLibCheck I have seen a good bit, but I'm not sure it's an optimal default).
Also the defaults for most users, particularly beginners, may not necessarily be the best for very advanced users (who likely have their own configs), those are two different workflows. What's optimal for you is not necessarily optimal for someone else.

[slikts]

Ok, I'm confused -- I thought you meant using incremental as a way to speed up type-checking with tsc not actual production builds.

It's clearly not a production config since it emits only the declarations (as a cache for the incremental option to work), and the outDir is in .cache.

As for opinions: not all opinions are the same; they differ by being well-grounded and informed or not. Even facts require interpretation. I'm not saying anything that would be akin to stating my favorite color; it's about what would be common scenarios for the user. Having faster type checking and not having builds break because of reasons outside of the user's control benefits the common case. Having complex types in ambient declarations is an advanced case. The suggestion that skipLibCheck would be optimal for advanced cases or specific to my workflow is the opposite of what's being said.

Here's the PR adding skipLibCheck to CRA that states the same rationale, and it also links to issues where users ask for it due to performance reasons. I found one comment in their repo where a user turned on skipLibCheck due to needing to check the ambient declarations.

The TS repo has many examples of complex issues with 3rd party typings (random example). Here's a good SO answer detailing the reasons for it. The issue highlights type conflicts (different dependencies typing shared APIs), but there can be other issues; here's an odd error from my own library; it's unclear how to fix it since it uses a standard config and doesn't do anything special.

I'm looking at alternatives since these discussions are unnecessarily taxing; it shouldn't need to be explained that strictness shouldn't be an end in itself, particularly not in the context of popular tooling in a complex ecosystem.

[jaredpalmer]

This is a really good discussion. Learned a lot about TS!

A bit of history, when I made TSDX I made one change to how Formik’s build system worked: rpts2. OG pre-tsdx Formik used to use a two-step build (because Babel TS didn’t really exist yet). First it would use tsc and emit esnext js to a temp directory. Then rollup would bundle the temp directory to dist (so it could use Babel plugins). It’s very possible that I incorrectly kept around some TS config options that became redundant by switching to rpts2 but never noticed because “it just worked.”
—
FWIW, my course of action with libraries and apps differs with respect to incorrect dependency types. When writing an app, I will 100% use patch-package to fix the types. I want to check my deps. During dev/watch, I’m fine with foregoing dep checks on large projects, but prefer it if possible. During builds, I want as much type safety as possible. When making a lib, however, I don’t think patch package is acceptable. So I usually will fork the dep and publish my own corrected version or copy it into my libs source (with proper correction) if it’s small enough. Until reading this thread, I never considered skiplibCheck for libs, but maybe I should have. I usually err on the side of strictness and correctness over build performance.
—-
I think the question we want to answer is what the default behavior should be for novice and intermediate typescript folks. We will obviously have to document this somewhere in an “advanced config” section

cc’ing typescript team for their suggestion @orta

tl;dr we are debating what defaults should be for noEmit, skiplibCheck, incremental builds, and emitDeclaration given how rollup-plugin-typescript2 works.