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

Hoist propTypes for all wrapper components #432

Closed
levithomason opened this issue Aug 24, 2016 · 2 comments
Closed

Hoist propTypes for all wrapper components #432

levithomason opened this issue Aug 24, 2016 · 2 comments

Comments

@levithomason
Copy link
Member

levithomason commented Aug 24, 2016

This work is instrumental in facilitating the doc site component explorer and sandbox, see #427.

Some of our components are simple wrappers around others. When wrapping other components, all props are spread. Because since they did not access properties of the props object, propTypes definitions were not required by linting. See:

  1. Radio
  2. Header / HeaderH[1-6] Made obsolete by augmentation, see Breaking Changes: Add component augmentation #414.
  3. Textarea
  4. Select

We do however want to hoist the propTypes. Not only for better stack traces, but also for docs. Docs are generated from propTypes. Without any propTypes defined on the wrapper components, there are no docs.

Where Radio composes Checkbox, propTypes should be hoisted like so:

Radio.propTypes = {
  ...Checkbox.propTypes,
}
@levithomason
Copy link
Member Author

This will be fixed in #400. Forms refactored all form controls. The Header will be included there as well.

@levithomason
Copy link
Member Author

This issue is far less relevant now that augmentation exists. We cannot spread the propTypes of the component we are augmenting since it can be replaced by the user. The user can pass a component that has different props. Now we are validating for the default as component, but not for the actual rendered component.

For this reason, we can should only hoist prop types when the component cannot be changed. In our case, all components listed here can be changed.

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

No branches or pull requests

1 participant