Can't implement interface by throwing error #16874

Activity

1. DanielRosenwasser commented on Jun 30, 2017

   

   Daniel Rosenwasser (DanielRosenwasser)

   on Jun 30, 2017

   Last edited by DanielRosenwasser

   Member

   More actions

   Check out #16608 and #8767 for changes/discussion related to this. We could fix this with a voidWideningNever type, or by just checking for a contextual type.

   I'm only half-joking.
2. Daniel Rosenwasser (DanielRosenwasser)

   added

   In DiscussionNot yet reached consensusNot yet reached consensus

   SuggestionAn idea for TypeScriptAn idea for TypeScript

   on Jun 30, 2017
3. RyanCavanaugh commented on Jun 30, 2017

   

   Ryan Cavanaugh (RyanCavanaugh)

   on Jun 30, 2017

   Member

   More actions

   These should be identical:

   const o1: I = { m: function () { throw new Error('nyi'); } }
   const o2: I = { m() { throw new Error("not implemented"); } }

   👍React with 👍2Reacted by Aluan Haddad and Alexander Mills
4. masaeedu commented on Jul 9, 2017

   

   Asad Saeeduddin (masaeedu)

   on Jul 9, 2017

   Contributor

   More actions

   Daniel Rosenwasser (@DanielRosenwasser) Probably need more input from other people but I think it'd be better to have a flag that undoes #8767 rather than a mashup type of void and never.

   I'm actually fine with getting an error in this case:

   class Base {
       overrideMe() {
           throw new Error("You forgot to override me!");
       }
   }
   
   class Derived extends Base {
       overrideMe() {
           // Code that actually returns here
       }
   }

   From my perspective, it's better to have to explicitly annotate the return type in the base class, because it might be the case that my to-be-overridden function is actually supposed to return string, or an array, or any number of other things, and an inferred return type of void will just mislead someone who has a reference of type Base.
5. RyanCavanaugh commented on Jul 9, 2017

   

   Ryan Cavanaugh (RyanCavanaugh)

   on Jul 9, 2017

   Member

   More actions

   Asad Saeeduddin (@masaeedu) we talked about this quite a bit last week. The problem with erroring in the above code is that it would be a breaking change. The only reason we even did #8767 was because the number of breaks it induced was too significant.

   Breaking change concerns aside, the good thing with giving a type void here is that no one can meaningfully misuse a void value obtained via a Base instance reference. Conversely, you can trivially misuse a never value because it's e.g. a valid function argument to any parameter position. It's worth considering the case where the author of Base does not have any Deriveds in their codebase (because they're shipping a base class library) -- they won't realize they have a "missing" type annotation until one of their consumers shows up complaining.

   Re: another flag... this is not something that would be worth doubling the configuration space of TypeScript over.

   👍React with 👍1Reacted by Asad Saeeduddin
6. ghost closed this as completedin #20052on Jan 5, 2018
7. Mohamed Hegazy (mhegazy)

   added

   FixedA PR has been merged for this issueA PR has been merged for this issue

   and removed

   In DiscussionNot yet reached consensusNot yet reached consensus

   on Jan 6, 2018
8. Mohamed Hegazy (mhegazy)

   added this to the TypeScript 2.7 milestone on Jan 6, 2018
9. microsoft

   locked and limited conversation to collaborators on Jul 3, 2018