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

Deduplicate yarn packages #1549

Merged
merged 8 commits into from
Nov 19, 2019
Merged

Conversation

jeanregisser
Copy link
Contributor

@jeanregisser jeanregisser commented Oct 31, 2019

Description

yarn deduplication is currently not optimal. After a while of adding/updating/removing packages, some duplicated versions are installed even if they have compatible semver ranges!

See yarnpkg/yarn#3778 and https://medium.com/@bnaya/yarn-deduplicate-the-hero-we-need-f4497a362128

Until yarn fixes this (hopefully in yarn v2) we can use this tool https://github.com/atlassian/yarn-deduplicate to limit this duplication.

For this PR, I ran the following: npx yarn-deduplicate yarn.lock && yarn

Here are the results on my machine (late 2013 MacBook Pro):

  • Total unique packages in yarn.lock went from 4307 to 3868 (~10% reduction).
  • Size of the repo after install went from 3.8G to 3.4G (saved ~400MB, also ~10% reduction)
  • yarn install time with a clean yarn cache (yarn reset has been run) went from 3m46s to 3m20s (~8% reduction)
    This includes download time of the packages which may vary depending on network conditions.
  • yarn install time with a populated yarn cache (yarn reset-modules has been run, so node_modules are empty) went from 2m29s to 2m0s (~20% reduction).
    This doesn't require downloading packages since the local yarn cache is populated.

Important: this changes our dependency tree. There are certain code paths that now will run with a different set of dependencies.

Tested

Before the changes

Unique packages in yarn.lock:

❯ grep "\sversion \"" yarn.lock | wc -l
    4307

1st run yarn install with a clean yarn cache (yarn reset has been run before starting this):
asciicast
2nd run yarn install with a populated yarn cache (yarn reset-modules has been run before starting this):
asciicast

After the changes

Unique packages in yarn.lock:

❯ grep "\sversion \"" yarn.lock | wc -l
    3868

1st run yarn install with a clean yarn cache (yarn reset has been run before starting this):
asciicast
2nd run yarn install with a populated yarn cache (yarn reset-modules has been run before starting this):
asciicast

Backwards compatibility

Yes.

Copy link
Contributor

@jmrossy jmrossy left a comment

Choose a reason for hiding this comment

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

Amazing, good idea!
If the tests pass then let's give it a try. if things break we can revert

@codecov
Copy link

codecov bot commented Oct 31, 2019

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #1549    +/-   ##
========================================
  Coverage   74.26%   74.26%            
========================================
  Files         277      277            
  Lines        7617     7617            
  Branches      669      953   +284     
========================================
  Hits         5657     5657            
  Misses       1845     1845            
  Partials      115      115
Flag Coverage Δ
#mobile 74.26% <100%> (ø) ⬆️
Impacted Files Coverage Δ
...bile/src/verify/VerificationInterstitialScreen.tsx 90.47% <100%> (ø) ⬆️
packages/mobile/src/web3/saga.ts 38.97% <100%> (ø) ⬆️
...ages/mobile/src/verify/VerificationInputScreen.tsx 64.51% <100%> (ø) ⬆️

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 3b15c1d...a74e879. Read the comment docs.

@martinvol
Copy link
Contributor

Awesome!

@martinvol martinvol closed this Oct 31, 2019
@martinvol martinvol reopened this Oct 31, 2019
@jeanregisser jeanregisser force-pushed the jeanregisser/dedup-packages branch 2 times, most recently from 3488435 to 86108d2 Compare November 4, 2019 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Have PR merge automatically when checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants