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

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

Closed
2 of 4 tasks
petamoriken opened this issue Aug 22, 2024 · 13 comments
Closed
2 of 4 tasks

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

petamoriken opened this issue Aug 22, 2024 · 13 comments
Assignees
Labels
needs investigation requires further investigation before determining if it is an issue or not
Milestone

Comments

@petamoriken
Copy link
Contributor

petamoriken commented Aug 22, 2024

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

Any thoughts?

@0f-0b
Copy link
Contributor

0f-0b commented Aug 23, 2024

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

  • “Recommended” means that the tsconfig reference recommends enabling the option.
  • “Independent” means that any project can be updated to enable the option independently of its dependencies (unless deno check --all is used).
  • “Has quick fix” means that the language server provides quick fixes for errors caused by enabling the option.
Recommended Independent Has quick fix
exactOptionalPropertyTypes
noImplicitOverride
noUncheckedIndexedAccess
useUnknownInCatchVariables

@lucacasonato
Copy link
Member

lucacasonato commented Aug 26, 2024

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
Copy link

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
Copy link
Member

dsherret commented Sep 5, 2024

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.

dsherret added a commit that referenced this issue Sep 5, 2024
@petamoriken
Copy link
Contributor Author

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
Copy link
Contributor

jsejcksn commented Sep 5, 2024

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
Copy link

imcotton commented Sep 5, 2024

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

@jsejcksn
Copy link
Contributor

jsejcksn commented Sep 6, 2024

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

@petamoriken
Copy link
Contributor Author

petamoriken commented Sep 10, 2024

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
Copy link
Member

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

@bartlomieju bartlomieju assigned bartlomieju and unassigned dsherret Sep 17, 2024
@petamoriken
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@petamoriken
Copy link
Contributor Author

I have reconsidered that this issue should be closed after #11889 (comment) has been resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs investigation requires further investigation before determining if it is an issue or not
Projects
None yet
Development

No branches or pull requests

8 participants