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

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 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() {
                    ~~~

Fix is up at #22935

Nathan Shively-Sanders (@sandersn)

Nathan Shively-Sanders (@sandersn) using the nightly I'm still getting the error

mohsen1/ts-bug-instance-member#1

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

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.

Mohsen Azimi (@mohsen1)

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

Nathan Shively-Sanders (@sandersn)

Thank you for prompt response Nathan Shively-Sanders (@sandersn)!

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() {
     ~~~~

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.

OK, #23137 improves the isMethodLike check.

Navigation Menu

Repository navigation

Extending a class method with prototype assignment confuses the compiler about membership type #22895

Bug Repository

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Extending a class method with prototype assignment confuses the compiler about membership type #22895

Description

Bug Repository

Activity

sandersn commented on Mar 27, 2018

mohsen1 commented on Apr 1, 2018

sandersn commented on Apr 2, 2018

sandersn commented on Apr 2, 2018

sandersn commented on Apr 2, 2018

sandersn commented on Apr 2, 2018

mohsen1 commented on Apr 2, 2018

mohsen1 commented on Apr 4, 2018

sandersn commented on Apr 4, 2018

sandersn commented on Apr 4, 2018

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions