microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.32k stars 12.53k forks source link

should error on assignment that shadows a prototype method #60649

Open kirkwaiblinger opened 2 days ago

kirkwaiblinger commented 2 days ago

🔎 Search Terms

own property, shadow, prototype, method, field, inheritance

🕗 Version & Regression Information

⏯ Playground Link

https://www.typescriptlang.org/play/?ts=5.8.0-dev.20241201#code/MYGwhgzhAEBiD29oG8BQ0PWPAdhALgE4Cuw+8hAFAJQrqYP4AWAlhAHQAOh85+AnpwCmAWSHN4AE2gBeaDVkA+Og1WZseeCCHsQ8AOaUA5BCZhJ8AO4xuvXoKHQAtuKZSj1ANz01AXx-Q-gy2fA5iEpIKaGoYGhBaOnqGRi4R0LjQIfbCHt4M-r5AA

💻 Code

class Foo {
    constructor() {
        this.prototypeMethod = () => {
            console.log('shadows prototype method');
        }
    }
    prototypeMethod() {
        console.log('method on prototype');
    }
}

🙁 Actual behavior

no error

🙂 Expected behavior

error - something like "Type 'Foo' has no instance property named 'prototypeMethod'. If you meant to reassign the method 'prototypeMethod', it must be a function-valued instance property instead of a prototype method.".

I expect an error here because the code is effectively equivalent to

class Foo {
    prototypeMethod = () => {
        console.log('shadows prototype method');
    };
    prototypeMethod() {
        console.log('method on prototype');
    }
}

which is a TS error (see https://github.com/microsoft/TypeScript/issues/13141).

Furthermore, it breaks error reporting in subclasses. The following code correctly reports a TS error due to the extended class not being able to override an instance property with a method: (playground)

class Foo {
    instanceProperty = () => {
        console.log('function-valued instance property');
    }
}

class Bar extends Foo {
    // TS ERROR: Class 'Foo' defines instance member property 'instanceProperty', but extended class 'Bar' defines it as instance member function.(2425)
    instanceProperty() {
        console.log('overridden with method')
    }
}

const b = new Bar();
b.instanceProperty(); // prints 'function-valued instance property'

However, this does not (playground)

class Foo {
    constructor() {
        this.method = () => {
            console.log('function-valued instance property')
        }
    }
    method() {
        console.log('method');
    }

}

class Bar extends Foo {
    // no error here  :(
    method() {
        console.log('overridden with method')
    }
}

const b = new Bar();
b.method(); // prints 'function-valued instance property'

Additional information about the issue

inspired by https://github.com/typescript-eslint/typescript-eslint/issues/10427

cc @miguel-leon

bradzacher commented 1 day ago

It's worth noting that this is an error in flow. It doesn't error for the reasons kirk mentioned though and instead errors because Flow treats all methods as readonly always to prevent unsafe / hard to analyse changes to classes like this.