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

Add support for complex properties #28

Closed

Conversation

jamesreggio
Copy link
Contributor

Simple assignment to copy statics was resulting in loss of behavior for complex properties (i.e., those with getter and setter methods).

This commit switches to using Object.defineProperty() to perform the copy, which preserves the behavior of these complex properties.

In case you're curious, here's a simplified use case that reflects how I discovered this bug. (Dimensions is not ready at the time that this file is loaded, which means that these static sizes must be calculated at read time.)

import React, {Component} from 'react';
import {Dimensions} from 'react-native';
import Header from './Header';

class Body extends Component {
  static get initialHeight() {
    const headerHeight = Header.initialHeight;
    return Dimensions.get('window').height - headerHeight;
  }

  render() {
    // ...
  }
}

Simple assignment to copy statics was resulting in loss of behavior
for complex properties (i.e., those with getter and setter methods).

This commit switches to using Object.defineProperty() to perform the
copy, which preserves the behavior of these complex properties.
@jamesreggio
Copy link
Contributor Author

Eh, nevermind.

static properties are not enumerable per the ES6 spec, since they're treated as methods. (I find this very perplexing.)

@mridgway
Copy link
Owner

My understanding is that the current draft has static properties as enumerable (old drafts did not): https://tc39.github.io/proposal-class-public-fields/#initialize-public-static-fields

I have not worked with complex static properties however, so I am interested in learning more about the interaction there.

@jamesreggio
Copy link
Contributor Author

You're correct — the current draft makes static fields enumerable. However, static methods and properties are actually already supported in ES6 and both are marked as non-enumerable.

To summarize for posterity:

class Foo {
  // non enumerable
  static getHeight() {
    return 42;
  }

  // non enumerable
  static get heightProperty() {
    return 42;
  }

  // enumerable
  static heightField = 42;
}

// all evaluate to 42
Foo.getHeight();
Foo.heightProperty;
Foo.heightField;

I find it perplexing that the 'complex' property isn't enumerable since it behaves like a field, but c'est la vie.

@mridgway
Copy link
Owner

mridgway commented Jul 6, 2017

Huh, that is really unfortunate... I feel like this might be a significant regression by switching to enumerables only.

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

Successfully merging this pull request may close these issues.

2 participants