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

Fully support ES6 Getters with Class Syntax #4595

Closed
ChadKillingsworth opened this issue May 12, 2017 · 3 comments
Closed

Fully support ES6 Getters with Class Syntax #4595

ChadKillingsworth opened this issue May 12, 2017 · 3 comments
Assignees
Labels

Comments

@ChadKillingsworth
Copy link
Contributor

ES6 getters provide an excellent (and standardized) alternative to computed properties. While the change notification is no longer automatic, static analysis and refactoring tools are supported.

Currently to properly support ES6 getters you have to modify internal private properties:

function enableGetters(Superclass) {
  return class extends Superclass {
    /**
     * Overrides the default `Polymer.PropertyEffects` to ensure
     * ES6 getter properties are supported 
     *
     * @override
     */
    _initializeProperties() {
      const proto = Object.getPrototypeOf(this);
      proto['__dataHasAccessor'] = this['__dataHasAccessor'] || {};
      const getterProps = new Set();

      // Mark any getters on the prototype so that Polymer doesn't add it's own
      // This has to be done before calling super
      const propNames = Object.getOwnPropertyNames(proto);
      propNames.forEach(propName => {
        const descriptor = Object.getOwnPropertyDescriptor(proto, propName);
        if (!(descriptor && descriptor.get)) {
          return;
        }

        // Indicate to Polymer that this property already has an accessor function
        // so that Polymer doesn't add another one.
        proto['__dataHasAccessor'][propName] = true;
        getterProps.add(propName);
      });

      super._initializeProperties();

      // For every getter, add a propagate effect
      getterProps.forEach(propName => {
        this._addPropertyEffect(
          propName,
          this.PROPERTY_EFFECT_TYPES.PROPAGATE,
          {
            'fn': (elem, propName) => {
                elem['__data'][propName] = elem[propName];
            }
          });
      });
    }
  }
}
@jfrazzano
Copy link

jfrazzano commented May 12, 2017 via email

@sorvell
Copy link
Contributor

sorvell commented Sep 14, 2017

Sorry for the delay on this. We'd really like to address this issue but think it may not make sense to layer on top of the current data binding system. There are a number of issues:

  1. the getter is not always used in the data-binding system. Instead the code often just looks for data in the private __data storage.

  2. the setter is not always used in the data-binding system. API like setProperties and _setPendingProperty, for example, do not directly call the property setter.

  3. For computed properties, polymer relies on the dependencies if finds in the computed property section to know when the computed "getter" should be called and any properties bound to that updated.

  4. The contract in _propertiesChanged is that all property values are available in the first argument. This would not be the case for any manually defined getters. While probably not fatal, this is something to contemplate.

That said, if these issues can be addressed via a PR in a way that makes sense, we would consider it.

@TimvdLippe
Copy link
Contributor

Closing per #4815 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants