Skip to content

Prototype assignments count as method-like #23137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 4, 2018

Conversation

sandersn
Copy link
Member

@sandersn sandersn commented Apr 4, 2018

For the purposes of reporting prototype/instance property conflicts, the compiler previously assumed that only methods were prototype properties. 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 ???!!!? i warned you about classes bro!!!! i told you dog!

class Base {
  get x() { return 1 }
}
class Derived extends Base { }
Derived.prototype.x = 2
console.log(new Derived().x) // 1 ??/?? it keeps happening

However, in Javascript, any assignment directly to the prototype is a prototype property, as in the line Derived.prototype.x = 2. So there's no conflict when the base property is on the prototype and the derived one is too:

class Base { }
Base.prototype.mustImplement = null
class Derived extends Base {
  mustImplement() {
    return 101
  }
}

This abstract-like pattern appears in webpack. Note that "isMethodLike" should really be "isPrototypeProperty" but the compiler is not very good at tracking this distinction. I optimistically renamed the function and documented its limitations in a comment.

Fixes #22895 (for real this time)

For the purposes of reporting prototype/instance property conflicts
@sandersn sandersn merged commit c4a504b into master Apr 4, 2018
@sandersn sandersn deleted the js/prototype-assignments-count-as-methodlike branch April 4, 2018 18:03
@microsoft microsoft locked and limited conversation to collaborators Jul 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant