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

feat: Update, cleanup and move necessary dependencies to peerDependencies #485

Open
wants to merge 9 commits into
base: alpha
from

Conversation

@voronianski
Copy link
Member

commented Oct 5, 2019

Addresses #453.

Approach and changes

  • move react, react-dom, @emotion/core, @emotion/styled and emotion-theming to package peerDependencies
  • move necessary dev dependencies from dependencies to devDependencies
  • update dependencies to latest minor and then to latest major whenever it's possible (by checking their release notes for breaking changes)
  • update code where it was needed to support updated dependencies
  • update deprecated storybook methods in all stories
  • run all test and build commands without errors

Questions

Not updated dependencies

Only 2 dependencies were not upgraded to latest:

commitizen                        latest  3.1.2  ❯  4.0.3  https://github.com/commitizen/cz-cli
react-docgen-annotation-resolver  latest  1.1.0  ❯  2.0.0  https://github.com/Jmeyering/react-docgen-annotation-resolver#readme
  • commitizen in their latest v4 removed support of Node v8 completely
  • react-docgen-annotation-resolver in its' lates v2 brings support for react-docgen v5 which is in beta yet

Actually updating commitizen seems to be not a problem in my opinion, as we're anyways developing Circuit being on latest version of Node.

Adding react and react-dom to devDependencies too?

Currently on yarn installation command you will get a tone of warnings from Circuit UI deps telling you about unmet peer dependencies react and react-dom, e.g:

warning " > downshift@3.3.4" has unmet peer dependency "react@>=0.14.9".
warning "downshift > @reach/auto-id@0.2.0" has unmet peer dependency "react@^16.8.0".
warning "downshift > @reach/auto-id@0.2.0" has unmet peer dependency "react-dom@^16.8.0".
warning " > markdown-to-jsx@6.10.3" has unmet peer dependency "react@>= 0.14.0".
warning " > react-dates@21.2.0" has unmet peer dependency "react@^0.14 || ^15.5.4 || ^16.1.1".
warning " > react-dates@21.2.0" has unmet peer dependency "react-dom@^0.14 || ^15.5.4 || ^16.1.1".
warning " > react-dates@21.2.0" has unmet peer dependency "react-with-direction@^1.3.1".
warning "react-dates > airbnb-prop-types@2.15.0" has unmet peer dependency "react@^0.14 || ^15.0.0 || ^16.0.0-alpha".
warning "react-dates > react-outside-click-handler@1.3.0" has unmet peer dependency "react@^0.14 || >=15".
warning "react-dates > react-outside-click-handler@1.3.0" has unmet peer dependency "react-dom@^0.14 || >=15".
warning "react-dates > react-portal@4.2.0" has unmet peer dependency "react@^15.0.0-0 || ^16.0.0-0 || ^17.0.0-0".
warning "react-dates > react-with-direction@1.3.1" has unmet peer dependency "react@^0.14 || ^15 || ^16".
warning "react-dates > react-with-direction@1.3.1" has unmet peer dependency "react-dom@^0.14 || ^15 || ^16".
warning "react-dates > react-with-styles@4.0.1" has unmet peer dependency "react@>=0.14".
warning " > react-event-listener@0.6.6" has unmet peer dependency "react@^16.3.0".
warning " > react-modal@3.10.1" has unmet peer dependency "react@^0.14.0 || ^15.0.0 || ^16".
warning " > react-modal@3.10.1" has unmet peer dependency "react-dom@^0.14.0 || ^15.0.0 || ^16".
warning " > react-popper@1.3.4" has unmet peer dependency "react@0.14.x || ^15.0.0 || ^16.0.0".
warning "react-popper > create-react-context@0.3.0" has unmet peer dependency "react@^0.14.0 || ^15.0.0 || ^16.0.0".
warning " > react-text-mask@5.4.3" has unmet peer dependency "react@^0.14.0 || ^15.0.0 || ^16.0.0".
warning " > react-transition-group@4.3.0" has unmet peer dependency "react@>=16.6.0".
warning " > react-transition-group@4.3.0" has unmet peer dependency "react-dom@>=16.6.0".
warning " > recompose@0.30.0" has unmet peer dependency "react@^0.14.0 || ^15.0.0 || ^16.0.0".
warning "@storybook/addon-a11y > @storybook/components@5.2.1" has unmet peer dependency "react-dom@*".
warning "@storybook/addon-a11y > @storybook/theming@5.2.1" has unmet peer dependency "react-dom@*".
warning "@storybook/addon-a11y > react-sizeme@2.6.7" has unmet peer dependency "react-dom@^0.14.0 || ^15.0.0-0 || ^16.0.0".
warning "@storybook/addon-links > @storybook/router@5.2.1" has unmet peer dependency "react@*".
warning "@storybook/addon-links > @storybook/router@5.2.1" has unmet peer dependency "react-dom@*".
warning "@storybook/addon-storysource > react-syntax-highlighter@8.1.0" has unmet peer dependency "react@>= 0.14.0".
warning " > @storybook/addon-info@5.2.1" has unmet peer dependency "react@*".
warning "@storybook/addon-info > react-element-to-jsx-string@14.1.0" has unmet peer dependency "react-dom@^0.14.8 || ^15.0.1 || ^16.0.0".
warning " > @storybook/addon-jest@5.2.1" has unmet peer dependency "react@*".
warning " > @storybook/addon-knobs@5.2.1" has unmet peer dependency "react@*".
warning "@storybook/addon-knobs > react-select@3.0.8" has unmet peer dependency "react@^16.8.0".
warning "@storybook/addon-knobs > react-select@3.0.8" has unmet peer dependency "react-dom@^16.8.0".
warning "@storybook/addon-knobs > react-color > @icons/material@0.2.4" has unmet peer dependency "react@*".
warning "@storybook/addon-knobs > react-select > react-input-autosize@2.2.2" has unmet peer dependency "react@^0.14.9 || ^15.3.0 || ^16.0.0-rc || ^16.0".
warning "@storybook/addon-knobs > react-select > react-transition-group@2.9.0" has unmet peer dependency "react@>=15.0.0".
warning "@storybook/addon-knobs > react-select > react-transition-group@2.9.0" has unmet peer dependency "react-dom@>=15.0.0".
warning " > @storybook/addon-links@5.2.1" has unmet peer dependency "react@*".
warning " > @storybook/addon-options@5.2.1" has unmet peer dependency "react@*".
warning " > @storybook/addon-storysource@5.2.1" has unmet peer dependency "react@*".
warning " > @storybook/react@5.2.1" has unmet peer dependency "react@*".
warning " > @storybook/react@5.2.1" has unmet peer dependency "react-dom@*".
warning "@storybook/react > @storybook/core@5.2.1" has unmet peer dependency "react@*".
warning "@storybook/react > @storybook/core@5.2.1" has unmet peer dependency "react-dom@*".
warning " > css-loader@3.2.0" has unmet peer dependency "webpack@^4.0.0".
warning " > @testing-library/react@9.3.0" has unmet peer dependency "react@*".
warning " > @testing-library/react@9.3.0" has unmet peer dependency "react-dom@*".
warning " > @testing-library/react-hooks@2.0.3" has unmet peer dependency "react@>=16.9.0".
warning " > component-playground@3.2.1" has unmet peer dependency "react@^15.6.0 || ^16.0.0".
warning " > component-playground@3.2.1" has unmet peer dependency "react-dom@^15.6.0 || ^16.0.0".
warning "component-playground > react-codemirror2@5.1.0" has unmet peer dependency "react@>=15.5 <=16.x".
warning " > docz@1.3.2" has unmet peer dependency "react@^16.8.0".
warning " > docz@1.3.2" has unmet peer dependency "react-dom@^16.8.0".
warning " > docz-theme-default@1.2.0" has unmet peer dependency "react@^16.8.0".
warning " > docz-theme-default@1.2.0" has unmet peer dependency "react-dom@^16.8.0".
warning " > react-test-renderer@16.10.2" has unmet peer dependency "react@^16.0.0".

Relatively simple fix will be to just add react and react-dom to devDependencies so the warnings will disappear. The downside of it could be that we will need to keep track of React versions in both lists of dependencies.

Definition of done

  • Development completed
  • Reviewers assigned
  • Unit and integration tests
  • Meets minimum browser support
  • Meets accessibility requirements
@voronianski voronianski added the tech label Oct 5, 2019
@voronianski voronianski added this to the v2.0 milestone Oct 5, 2019
@voronianski voronianski added this to In progress in v2 via automation Oct 5, 2019
@codecov

This comment has been minimized.

Copy link

commented Oct 5, 2019

Codecov Report

Merging #485 into alpha will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##            alpha     #485      +/-   ##
==========================================
+ Coverage   76.15%   76.16%   +<.01%     
==========================================
  Files         167      167              
  Lines        2453     2454       +1     
  Branches      435      435              
==========================================
+ Hits         1868     1869       +1     
  Misses        467      467              
  Partials      118      118
Impacted Files Coverage Δ
src/components/Sidebar/Sidebar.js 100% <ø> (ø) ⬆️
src/components/Tooltip/Tooltip.js 100% <ø> (ø) ⬆️
src/styles/style-helpers.js 98.36% <ø> (ø) ⬆️
.../components/AutoCompleteInput/AutoCompleteInput.js 84.37% <ø> (ø) ⬆️
...mponents/SideNav/components/Modal/isOverflowing.js 0% <ø> (ø) ⬆️
...omponents/SideNav/components/Modal/ModalManager.js 0% <ø> (ø) ⬆️
...ents/Sidebar/components/CloseButton/CloseButton.js 100% <100%> (ø) ⬆️
@voronianski voronianski changed the title tech: Update/cleanup dependencies and move necessary modules to peerDependencies feat: Update, cleanup and move necessary dependencies to peerDependencies Oct 7, 2019
@connor-baer connor-baer changed the base branch from canary to alpha Oct 7, 2019
Copy link
Member

left a comment

This is A M A Z I N G! 💯👏 Thank you so much for taking care of the Storybook deprecation as well. That must have been a lot of work.

Let me know if my feedback is clear. I know there's not much documentation out there around peer dependencies.

build/global-styles Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
Copy link
Member

left a comment

👍 for @connor-baer suggestions

Copy link
Member

left a comment

Thank you for fixing the peerDependencies! 👍

As we discussed on Slack, I'd like to split out the Storybook-related changes since they're not a breaking change and we need them for the migration to Storybook Docs. From what I can tell, those changes are mostly contained in these 2 commits:

I've cherry-picked those commits into a new branch: feature/storybook-docs. I'm not sure if it's easier to remove those commits from this PR (the first commit also contains dependency upgrades) or simply rebase on the new branch. WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
v2
  
In progress
3 participants
You can’t perform that action at this time.