Skip to content

Prototype assignments count as method-like#23137

Merged
sandersn merged 2 commits into
masterfrom
js/prototype-assignments-count-as-methodlike
Apr 4, 2018
Merged

Prototype assignments count as method-like#23137
sandersn merged 2 commits into
masterfrom
js/prototype-assignments-count-as-methodlike

Conversation

@sandersn
Copy link
Copy Markdown
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