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

[core] Deduplicate packages #16608

Merged
merged 4 commits into from
Jul 19, 2019
Merged

[core] Deduplicate packages #16608

merged 4 commits into from
Jul 19, 2019

Conversation

merceyz
Copy link
Member

@merceyz merceyz commented Jul 15, 2019

Output:

$ yarn upgrade @babel/core && node scripts/postinstall.js
yarn upgrade v1.17.3
[1/4] Resolving packages...
[2/4] Fetching packages...
info fsevents@1.2.9: The platform "win32" is incompatible with this module.
info "fsevents@1.2.9" is an optional dependency and failed compatibility check. Excluding it from installation.
[3/4] Linking dependencies...
warning " > @material-ui/pickers@3.1.1" has unmet peer dependency "@date-io/core@^1.3.6".
warning " > jss-rtl@0.2.3" has unmet peer dependency "jss@^9.0.0".
[4/4] Rebuilding all packages...
success Saved lockfile.
success Saved 9 new dependencies.
info Direct dependencies
└─ @babel/core@7.5.4
info All dependencies
├─ @babel/core@7.5.4
├─ @babel/helpers@7.5.4
├─ @babel/highlight@7.5.0
├─ ansi-styles@3.2.1
├─ chalk@2.4.2
├─ color-convert@1.9.3
├─ color-name@1.1.3
├─ js-tokens@4.0.0
└─ supports-color@5.5.0
Done in 16.74s.
22 duplicated package(s) found
  Package "@babel/core" wants ^7.1.6 and could get 7.5.4, but got 7.4.4
  Package "@babel/core" wants ^7.4.4 and could get 7.5.4, but got 7.4.4
  Package "@babel/generator" wants ^7.1.2 and could get 7.5.0, but got 7.4.4
  Package "@babel/generator" wants ^7.4.0 and could get 7.5.0, but got 7.4.4
  Package "@babel/helpers" wants ^7.1.2 and could get 7.5.4, but got 7.4.4
  Package "@babel/parser" wants ^7.0.0 and could get 7.5.0, but got 7.4.4
  Package "@babel/parser" wants ^7.1.2 and could get 7.5.0, but got 7.4.4
  Package "@babel/parser" wants ^7.1.6 and could get 7.5.0, but got 7.4.4
  Package "@babel/parser" wants ^7.4.3 and could get 7.5.0, but got 7.4.4
  Package "@babel/traverse" wants ^7.0.0 and could get 7.5.0, but got 7.4.4
  Package "@babel/traverse" wants ^7.1.0 and could get 7.5.0, but got 7.4.4
  Package "@babel/traverse" wants ^7.4.3 and could get 7.5.0, but got 7.4.4
  Package "@babel/types" wants ^7.1.2 and could get 7.5.0, but got 7.4.4
  Package "@babel/types" wants ^7.2.0 and could get 7.5.0, but got 7.4.4
  Package "@babel/types" wants ^7.3.0 and could get 7.5.0, but got 7.4.4
  Package "@babel/types" wants ^7.4.0 and could get 7.5.0, but got 7.4.4
  Package "ms" wants ^2.0.0 and could get 2.1.2, but got 2.1.1
  Package "resolve" wants ^1.10.0 and could get 1.11.1, but got 1.10.1
  Package "resolve" wants ^1.10.1 and could get 1.11.1, but got 1.10.1
  Package "resolve" wants ^1.4.0 and could get 1.11.1, but got 1.10.1
  Package "resolve" wants ^1.5.0 and could get 1.11.1, but got 1.10.1
  Package "resolve" wants ^1.8.1 and could get 1.11.1, but got 1.10.1
Deduplicating package(s)
yarn install v1.17.3
[1/4] Resolving packages...
[2/4] Fetching packages...
info fsevents@1.2.9: The platform "win32" is incompatible with this module.
info "fsevents@1.2.9" is an optional dependency and failed compatibility check. Excluding it from installation.
[3/4] Linking dependencies...
warning " > @material-ui/pickers@3.1.1" has unmet peer dependency "@date-io/core@^1.3.6".
warning " > jss-rtl@0.2.3" has unmet peer dependency "jss@^9.0.0".
[4/4] Building fresh packages...

$ node scripts/postinstall.js
Done in 12.57s.

@mui-pr-bot
Copy link

mui-pr-bot commented Jul 15, 2019

No bundle size changes comparing 42f78c6...e51c925

Generated by 🚫 dangerJS against e51c925

@merceyz merceyz changed the title [core] Package maintenance [core] Check duplicated packages and sync package.json files Jul 15, 2019
@merceyz merceyz changed the title [core] Check duplicated packages and sync package.json files [core] Check for duplicated packages and sync package.json files Jul 15, 2019
@eps1lon
Copy link
Member

eps1lon commented Jul 15, 2019

Can't lerna sync those?

Updating dependencies should probably be handled by a bot. Though it should only run weekly given the amount of packages in the workspace.

@merceyz
Copy link
Member Author

merceyz commented Jul 15, 2019

Can't lerna sync those?

As far as I can tell, no.

Updating dependencies should probably be handled by a bot. Though it should only run weekly given the amount of packages in the workspace.

Probably, but that isn't the goal of this PR.
I didn't update(yarn upgrade) anything

@codecov

This comment has been minimized.

@merceyz merceyz changed the title [core] Check for duplicated packages and sync package.json files [core] Automatically deduplicate packages Jul 16, 2019
@merceyz merceyz marked this pull request as ready for review July 16, 2019 15:27
package.json Outdated Show resolved Hide resolved
@merceyz merceyz changed the title [core] Automatically deduplicate packages [core] Deduplicate packages Jul 17, 2019
@merceyz
Copy link
Member Author

merceyz commented Jul 17, 2019

@eps1lon I've added it into the CI task and moved it away from postinstall.

To deduplicate packages one can now use yarn deduplicate.

Example of a CI failure where there was duplicated packages: https://circleci.com/gh/mui-org/material-ui/106720?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

Isn't it simpler to run yarn yarn-deduplicate in CI and check if changes occurred? Basically just move it between install and "Should not have any git not staged".

I don't see anything meaningful being done in scripts/deduplicate

@merceyz
Copy link
Member Author

merceyz commented Jul 18, 2019

Only benefit it provides locally is that it runs yarn install for you while on the CI it doesn't write to the disk and provides a more useful error message.

@oliviertassinari
Copy link
Member

Both approaches sound OK, we had this duplication problems surface twice in the past on dependencies upgrade PR, great to see it fixed systematically. @eps1lon, your call.

@eps1lon eps1lon self-requested a review July 19, 2019 18:37
Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

Makes more sense to have all steps in a single script file. Nice job!

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.

4 participants