Throw an error when functions on statics clash due to duplicate keys

[charliermarsh]

Fixes #1947.

I agree with @spicyj and find this behavior confusing, so I thought I'd send in a pull request. (This is my first PR for React--did my best to follow the contributing guidelines!)

[sophiebits]

I like this.

[charliermarsh]

One thing I've noticed: Before this change, if you had a single mixin with its own getDefaultProps on statics, then the result of that call would be merged with the getDefaultProps of the base component. If you had two mixins that both defined their own getDefaultProps on their statics, then those functions would be chained, returning undefined, and so neither of them would have any influence on the base component's getDefaultProps.

After this change, the single mixin behavior remains the same. But if you have two mixins with their own getDefaultProps on statics, you get an error (which is probably more helpful than returning undefined, as they were before). Generally, do we want to be merging the result of each mixin's getDefaultProps?

[sophiebits]

I know I told you that getDefaultProps in statics should work but now that I'm looking again, I'm not sure if that was actually intended:

  /**
   * Special case getDefaultProps which should move into statics but requires
   * automatic merging.
   */

[charliermarsh]

There's also a note above RESERVED_SPEC_KEYS:

/**
 * ...
 * Despite being static, they must be defined outside of the "statics" key
 * under which all other static methods are defined.
 */

Suggests to me that a getDefaultProps method defined on statics shouldn't be merged in.

[charliermarsh]

As far as I can tell, (post-change,) putting statics in getDefaultProps only works because statics happens to get mixed into the spec before getDefaultProps. We could either force a certain order of iteration or (my preference) fail in mixStaticSpecIntoComponent if they try to add a key from RESERVED_SPEC_KEYS.

[sophiebits]

For now, maybe it's best to throw an error if getDefaultProps is in statics.

[charliermarsh]

(Somewhat relevant: #1601. In this case, we'd want to make sure that the RESERVED_SPEC_KEYS get mixed in before any mixins. I still prefer throwing an error when you use a RESERVED_SPEC_KEY in statics, though.)

[sophiebits]

Although you'll probably be very confused if you use componentDidMount, etc. as static properties, I don't think there's a real reason to disallow it – I think getDefaultProps is the only problematic one because that gets stored on the constructor.

[charliermarsh]

@spicyj Don't all the properties in RESERVED_SPEC_KEY get stored on the constructor? (componentDidMount is in ReactCompositeComponentInterface but not RESERVED_SPEC_KEY).

[sophiebits]

Yes, I'm an idiot. Carry on.