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

Remove direct react dependency, cleanup unused dependencies, upgrade webpack-cli + sass #152

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

acusti
Copy link

@acusti acusti commented Oct 12, 2021

the direct react and react-dom dependencies cause react and react-dom 16 to be installed when adding react-code-input to a react v17.x application, so i removed the direct react and react-dom dependencies, keeping them only as peer dependencies (fixes #75). storybook still takes care of installing them so that npm start works, and yarn build still works. in addition:

@codecov
Copy link

codecov bot commented Oct 12, 2021

Codecov Report

Merging #152 (fe4ae2d) into master (2599c5d) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #152   +/-   ##
=======================================
  Coverage   96.55%   96.55%           
=======================================
  Files           2        2           
  Lines         145      145           
  Branches       41       41           
=======================================
  Hits          140      140           
  Misses          5        5           
Impacted Files Coverage Δ
src/ReactCodeInput.js
src/utils.js
ReactCodeInput.js 96.45% <0.00%> (ø)
utils.js 100.00% <0.00%> (ø)

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 2599c5d...fe4ae2d. Read the comment docs.

Comment on lines -70 to -71
"react": "^16.3.2",
"react-dom": "^16.3.2"
Copy link

Choose a reason for hiding this comment

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

These should still be put into devDependencies to show they are directly used by this package, relying on other packages to install your dependencies isn't as good and when using Yarn PnP won't even work as you must declare explicitly all dependencies you import in your package.

@austinbiggs
Copy link

Bump! Is there any chance that this will be merged?

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.

Install fails on Node version v12.6.0 package.json misconfigured
3 participants