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

fix: treeshake Downshift and useSelect #850

Merged
merged 4 commits into from
Dec 11, 2019

Conversation

silviuaavram
Copy link
Collaborator

@silviuaavram silviuaavram commented Dec 9, 2019

defaultProps and stateChangeTypes prevent Downshift from being tree-shakeable. Also the prop type checks in useSelect.

Use no-side-effect-class-properties in .babelrc.js along with the original configs from kcd-scripts. https://github.com/naver/babel-plugin-no-side-effect-class-properties @kentcdodds maybe you want to include this as well or mark it to be added once a more official version is released.

Updated rollup.config.js to use the babel config file.

Only check hook prop-types if not on production.

Result: treeshakeable Downshift.

Most grateful to @layershifter for contributing with this.

babel/babel#8592
babel/babel#6963

@silviuaavram silviuaavram changed the title Chore: treeshake downshift chore: treeshake downshift Dec 9, 2019
const validatePropTypes =
process.env.NODE_ENV === 'production'
? noop
: getPropTypesValidator(useSelect, propTypes)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rather recommend to use a condition in useSelect as in this case validatePropTypes function will be unused and removed:

if (process.env.NODE_ENV !== 'production') {
  validatePropTypes(userProps)
}

@silviuaavram
Copy link
Collaborator Author

@all-contributors please add @layershifter with code, infra, ideas

@allcontributors
Copy link
Contributor

@silviuavram

I've put up a pull request to add @layershifter! 🎉

@silviuaavram
Copy link
Collaborator Author

@all-contributors please add @epiqueras with ideas

@allcontributors
Copy link
Contributor

@silviuavram

I've put up a pull request to add @epiqueras! 🎉

@silviuaavram silviuaavram changed the title chore: treeshake downshift fix: treeshake Downshift and useSelect Dec 10, 2019
@codecov-io
Copy link

Codecov Report

Merging #850 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #850   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           9      8    -1     
  Lines         724    702   -22     
  Branches      156    158    +2     
=====================================
- Hits          724    702   -22
Impacted Files Coverage Δ
src/hooks/useSelect/index.js 100% <100%> (ø) ⬆️
src/downshift.js 100% <0%> (ø) ⬆️
src/hooks/useSelect/stateChangeTypes.js

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e3a0c23...ee24139. Read the comment docs.

@silviuaavram silviuaavram merged commit ae49ad8 into master Dec 11, 2019
@silviuaavram silviuaavram deleted the chore/treeshake-downshift branch December 11, 2019 11:34
@silviuaavram
Copy link
Collaborator Author

🎉 This PR is included in version 3.4.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

None yet

3 participants