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

Make defaultProps static #169

Open
1 task done
selvagsz opened this issue Jul 23, 2018 · 1 comment
Open
1 task done

Make defaultProps static #169

selvagsz opened this issue Jul 23, 2018 · 1 comment

Comments

@selvagsz
Copy link

selvagsz commented Jul 23, 2018

FEATURE REQUEST / CODE CHANGE

  • I have deleted the BUG REPORT section

Summary

Migrate the instance.getDefaultProps() to static Klass.defaultProps

Motivation

  1. Stays more in alignment with the inspired react apis
  2. I don't think we should rely on instance for providing default values for a component
  3. I'd personally prefer composition over inheritance.
  4. The static syntax looks more obvious & cleaner than the getDefaultProps hook. Can be separated from the implementation if required which opens up more space for docs generation

Detailed design

import Component from '@ember/component';

const AlertBox = Component.extend({
  //
});

AlertBox.propTypes = {
  type: PropTypes.oneOf(['warning', 'info', 'error', 'success']),
  dismissible: PropTypes.bool,
};

AlertBox.defaultProps = {
  type: 'warning',
  dismissible: true,
};

export default AlertBox;

How we teach this

This change would be breaking in two aspects

  1. removing the inheritance support (invoking _super via concatenated prop)
  2. no this context while providing the defaultProps

So, a major version bump would be my call

Alternatives

  • Leave as it is
@selvagsz
Copy link
Author

I can help with a PR if accepted

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

No branches or pull requests

1 participant