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

Consider enabling "noImplicitOverride" by default #11836

Closed
Tracked by #25162
kitsonk opened this issue Aug 24, 2021 · 7 comments · Fixed by #25695
Closed
Tracked by #25162

Consider enabling "noImplicitOverride" by default #11836

kitsonk opened this issue Aug 24, 2021 · 7 comments · Fixed by #25695
Labels
cli related to cli/ dir suggestion suggestions for new features (yet to be agreed)
Milestone

Comments

@kitsonk
Copy link
Contributor

kitsonk commented Aug 24, 2021

In TypeScript 4.3, the override keyword was introduced which ensure when type checking code that a method with the override keyword is present in the base class. This helps make it more explicit when extending classes. In addition, there is the flag "noImplicitOverride" which enforces that if a method is present in a base class and the extended class that the override keyword is present in the extended class, helping ensure that users don't "accidentally" override a method unintentionally.

I am personally not sure about this, as it has a lot of potential to break existing code, but I wanted to bring it up so that we can try to consider what "version" of TypeScript we want to support in Deno. The existing behaviour allows people to opt-in to using override for the protection it offers, but essentially doesn't deliver as "safe" as code as possible.

@kitsonk kitsonk added cli related to cli/ dir suggestion suggestions for new features (yet to be agreed) labels Aug 24, 2021
@getspooky
Copy link
Contributor

I think it's a good to enable noImplicitOverride in order to to improve code health in the repository. but as you already said it will introduce a lot of code refactoring. A good example of noImplicitOverride integration is Angular.

@David-Else
Copy link

I think it is one of the best improvements they ever made! I always hated inheritance due to not being sure if the extended class method was overriding the base class. It would be great to enable noImplicitOverride.

@sigmaSd
Copy link
Contributor

sigmaSd commented Jun 22, 2023

for future searchers to enable this add to deno.json

{
  "compilerOptions": {
    "noImplicitOverride": true
  }
}

(take from denoland/std#1933)

@sigmaSd
Copy link
Contributor

sigmaSd commented Jun 22, 2023

Maybe this can be considered for deno 2.0 ?

@sigmaSd
Copy link
Contributor

sigmaSd commented Jun 22, 2023

though I just discovered this issue microsoft/TypeScript#47250 which makes this feature in my opinion partially finished

@lucacasonato
Copy link
Member

We're doing this for Deno 2.

@dsherret
Copy link
Member

I think this one hasn't been decided yet or implemented, so reopening.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants