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

Export select prop types #1876

Closed
wants to merge 4 commits into from
Closed

Export select prop types #1876

wants to merge 4 commits into from

Conversation

ecbrodie
Copy link

@ecbrodie ecbrodie commented Jul 17, 2017

The purpose of this PR is to allow the prop types from the Select component to be exported. The use case that this satisfies is to allow an application using the react-select to build a custom wrapper around the Select component (ie, to allow for customizations that serve business needs of the application, such as specific default prop values), but not need to copy-paste prop type code from react-select to satisfy the prop-type checks. This duplicated prop-type code can simply be replaced with an import of SelectPropTypes from react-select.

I also made a couple other minor improvements, to hopefully improve the development experience for react-select contributors:

  • Adds an .nvmrc file. The README calls out that Node v4 should be used for development environments. Using this file enables better UX for contributors using nvm
  • Removes an outdated line from CONTRIBUTING.md

Feedback welcome. I hope this PR adds value to this project. Thank you.

UPDATE

One more thing I should note. I originally tried to export SelectPropTypes from the same file as the Select component, select.js. I did this as a named export, alongside the default export of Select. Unfortunately, this ended up breaking all tests of the Select component. I narrowed down the reason to behaviour of ES6 transpilation in this project. Before my changes (master branch), lib/Select.js would contain this line:

module.exports = exports['default'];

However, with the named export included, the export object would have its default and SelectPropTypes properties set accordingly, but that latter line for module.exports would not appear. I decided to work around this by keeping a single export (the default) in Select.js and extracting SelectPropTypes into its own file with an export default. I am open to hearing other suggestions to better export SelectPropTypes. This could also be a pattern for the prop types of other components for future PRs, as needed.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 94.043% when pulling f5dabf1 on ecbrodie:export_select_prop_types into 6c0ee59 on JedWatson:master.

@jochenberger
Copy link
Contributor

See also #1882

@ecbrodie
Copy link
Author

@JedWatson do you have any feedback on this PR? Thank you.

@jochenberger
Copy link
Contributor

@ecbrodie, can't you just use Select.propTypes?

@ecbrodie
Copy link
Author

The feeling when you look back at your own PR from 3 months ago and say "Shit, what was I thinking?"

You are right @jochenberger . propTypes are indeed exported. Derp.

@ecbrodie ecbrodie closed this Sep 19, 2017
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.

3 participants