Typescript: Extending a class method with prototype assignment confuses the compiler about membership type

[mohsen1]

This is an issue I encountered when working on https://github.com/webpack/webpack/pull/6862

Bug Repository

See the bug repo for complete code
https://github.com/mohsen1/ts-bug-instance-member

Run npm test to run the test

Expected behavior:
No Error
Actual behavior:

index.js:7:19 - error TS2424: Class 'Ex' defines instance member function 'foo', but extended class 'MyClass' defines it as instance member property.

7 MyClass.prototype.foo = function() {
                    ~~~

[sandersn]

Fix is up at #22935

[mohsen1]

@sandersn using the nightly I'm still getting the error

https://github.com/mohsen1/ts-bug-instance-member/pull/1

[sandersn]

I see, it looks like it breaks going the other direction, when the base uses prototype assignment. I’ll take anothe look.

[sandersn]

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 this isn't an instance of Ex. I'll have a fix up shortly; I see a couple of ways to do it.

[sandersn]

Note that --noImplicitThis works with "checkJs": true, but it marks every usage of this outside 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.

[sandersn]

OK, the fix is merged. @mohsen1 can you try typescript@next tomorrow?

[mohsen1]

Thank you for prompt response @sandersn!

[mohsen1]

Still seeing this error with latest TS nightly.

Run yarn tsc in 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() {
     ~~~~

[sandersn]

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 a notImplementedFunction that 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.

[sandersn]

OK, #23137 improves the isMethodLike check.