denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
97.24k stars 5.36k forks source link

suggestion: enable some compilerOptions by default in Deno v2 #25162

Closed petamoriken closed 1 month ago

petamoriken commented 2 months ago

I would like to enable the following options by default in Deno v2:

Any thoughts?

0f-0b commented 2 months ago

There are 3 aspects I think are important to consider. In the table below,

Recommended Independent Has quick fix
exactOptionalPropertyTypes
noImplicitOverride
noUncheckedIndexedAccess
useUnknownInCatchVariables
lucacasonato commented 2 months ago

I am in favor of enabling useUnknownInCatchVariables. I am also ok with enabling noImplicitOverride.

For exactOptionalPropertyTypes, we'd have to do some experimentation to see if it would be very breaking (for example checking std, fresh, some projects using fresh, lume, etc).

For noUncheckedIndexedAccess, I do not think we should enable it by default. The option is very prone to false positives unfortunately.

szagi3891 commented 2 months ago

I would suggest turning on all TypeScript checking options to the max. The only thing holding me back from using Deno right now is that the TypeScript checking is so poorly configured.

dsherret commented 1 month ago

I opened #25465 for useUnknownInCatchVariables.

For exactOptionalPropertyTypes and noUncheckedIndexedAccess I think it would be too breaking. For noImplicitOverride, I'm trying to figure out why TypeScript doesn't have this in the default "strict" options.

petamoriken commented 1 month ago

For exactOptionalPropertyTypes and noUncheckedIndexedAccess I think it would be too breaking.

Unfortunately I agree that "exactOptionalPropertyTypes" and "noUncheckedIndexedAccess" have too much impact on existing projects. How about enabling these options in new code by enabling them in deno init?

jsejcksn commented 1 month ago

For exactOptionalPropertyTypes and noUncheckedIndexedAccess I think it would be too breaking.

@dsherret What's wrong with making a breaking change at a new major version which encourages writing safer code? That seems like exactly the right time to me… and users can always modify their compiler options to suppress these safer checks. The alternative currently appears to me as "we can never enable safer type checking by default because people have already written less safe code and we don't want them to feel the need to make changes as part of a major version upgrade."

imcotton commented 1 month ago

or, is there any easy way to let me enable configs via https://github.com/tsconfig/bases?

jsejcksn commented 1 month ago

I would also like to include noImplicitReturns in this suggestion list — since it's not already enabled (📌)

petamoriken commented 1 month ago

It is possible to request to disable the options in compilerOptions for those who think the default settings are too strict in the release article, so I think it is better to make it as strict as possible. Especially for options that require type info that cannot be handled by deno_lint.

dsherret commented 1 month ago

I think noImplicitReturns should probably be duplicated in a lint rule and enabled by default.

petamoriken commented 1 month ago

I think noImplicitReturns should probably be duplicated in a lint rule and enabled by default.

It certainly seems so. This option need not be enabled by default, as it does not use any type information in particular. https://lint.deno.land/rules/explicit-function-return-type

petamoriken commented 1 month ago

Apparently it has been decided that "exactOptionalPropertyTypes" and "noUncheckedIndexedAccess" will not be enabled by default, so close this issue.

petamoriken commented 1 month ago

I have reconsidered that this issue should be closed after https://github.com/denoland/deno/issues/11889#issuecomment-2359386226 has been resolved.