Skip to content
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

Allow mixin classes to have members added via .prototype calls. #799

Merged
merged 3 commits into from
Dec 3, 2018

Conversation

aomarks
Copy link
Member

@aomarks aomarks commented Dec 1, 2018

For example:

  /** @type {string} */
  MyMixin.prototype.foo;

We already support this behavior for normal classes, but not for mixin classes.

We need this because we're going to change some class mixin properties such as PolymerElement._template to not be initialized in the constructor, but we still need a way to tell analyzer that they exist.

cc @sorvell @kevinpschaaf

Demonstrates problem where analyzer does not detect mixin class
properties that are added to the prototype.
For example:

  /** @type {string} */
  MyMixin.prototype.foo;

We already support this behavior for normal classes, but not for mixin
classes.
@rictic
Copy link
Contributor

rictic commented Dec 2, 2018

We can also do this via:

/** @type {string} */
this.foo;

in the constructor, which I'd argue is more legible. I guess if that's the only thing in the constructor it might be better for the VM to leave it off though?

@aomarks
Copy link
Member Author

aomarks commented Dec 2, 2018

We can also do this via:

/** @type {string} */
this.foo;

in the constructor, which I'd argue is more legible. I guess if that's the only thing in the constructor it might be better for the VM to leave it off though?

Yeah @kevinpschaaf and @sorvell specifically wanted to avoid having the property accesses at all in the constructor, for uncompiled users.

@aomarks aomarks merged commit ce5ca6c into master Dec 3, 2018
@aomarks aomarks deleted the mixin-prototype branch December 3, 2018 21:47
aomarks added a commit to Polymer/polymer that referenced this pull request Dec 6, 2018
Only significant change is from Polymer/tools#799
aomarks added a commit to Polymer/polymer that referenced this pull request Dec 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants