Repository navigation
Extending a class method with prototype assignment confuses the compiler about membership type #22895
Description
Activity
sandersn commented
on Mar 27, 2018 MemberMore actionsFix is up at #22935
- addedFixedA PR has been merged for this issueA PR has been merged for this issue
on Mar 28, 2018 Nathan Shively-Sanders (@sandersn) using the nightly I'm still getting the error
I see, it looks like it breaks going the other direction, when the base uses prototype assignment. I’ll take anothe look.
Well, actually it turns out that it's the arrow function being used as a method. This is ... weird, but technically correct as long as the author remembers that
thisisn't an instance ofEx. I'll have a fix up shortly; I see a couple of ways to do it.Reacted by Mohsen AzimiNote that
--noImplicitThisworks with"checkJs": true, but it marks every usage ofthisoutside a class, which makes it hard to use transitionally. So recognising arrow functions as methods is good for understanding existing code, but bad in that it doesn't prevent js authors from possibly buggy new code.I think the right thing is to prefer understanding existing code, since we hope people with write new code in Typescript. They will, at least, have years of experience lowering their expectations of the correctness of any new Javascript that they write.
OK, the fix is merged. Mohsen Azimi (@mohsen1) can you try
typescript@nexttomorrow?Reacted by Mohsen AzimiThank you for prompt response Nathan Shively-Sanders (@sandersn)!
Still seeing this error with latest TS nightly.
Run
yarn tscin this branch of Webpack to see errors:https://github.com/mohsen1/webpack/tree/bd1c827c884d943f68bbbf00cd91003875ae0bb2
sample:
lib/NormalModule.js:455:2 - error TS2425: Class 'Module' defines instance member property 'size', but extended class 'NormalModule' defines it as instance member function. 455 size() { ~~~~Yikes! OK, the problem this time is that the initialisers on the base aren't methods at all. They are
null, which is definitely not method-like. I think the intention is to document that they are "abstract", but without creating anotImplementedFunctionthat always throws.Upon re-reading the override checking code, it seems that the intent of the error is to prevent instance vs property confusion like this:
class Super { constructor() { this.p = () => 1 } } class Sub extends Super { p() { return 2 } } console.log(new Sub().p()) // 1 ???!!!? class Base { get x() { return 1 } } class Derived extends Base { } Derived.prototype.x = 2 console.log(new Derived().x) // 1 ??/?? it keeps happening
So "isMethodLike" should really be "isPrototypeProperty" but the compiler is not very good at tracking this distinction. Fortunately, for Javascript special assignments, we can just look at the left-hand-side of the assignment. I'll go make that change.
OK, #23137 improves the
isMethodLikecheck.Reacted by Mohsen Azimi- locked and limited conversation to collaborators
on Jul 25, 2018
This is an issue I encountered when working on webpack/webpack#6862
Bug Repository
See the bug repo for complete code
https://github.com/mohsen1/ts-bug-instance-member
Run
npm testto run the testExpected behavior:
No Error
Actual behavior: