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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unused dependencies #636

Merged
merged 14 commits into from Nov 4, 2019
Merged

Remove unused dependencies #636

merged 14 commits into from Nov 4, 2019

Conversation

emplums
Copy link

@emplums emplums commented Oct 24, 2019

Clean up, clean up!


I ran depcheck on Primer Components and found a bunch of unused deps! Removing them 馃帀

I also found a couple of other relics from the past that I removed:

  • the script folder which only contains the prepack script that's not being used anymore
  • the css.js file, associated dependencies, and rollup config for it. These styles were being imported in our old Next.js setup for our docs and aren't needed anymore :)

Also, react-dom was a dependency, but it only used in our tests (because some package we are using in our tests has it as a peerDep) so I moved it to devDeps. I think this is safe to do - the deploy site looks great and I double-triple checked that we aren't calling any ReactDOM methods or importing react-dom anywhere - but for some reason bundlephobia classifies it as a missing dependency 馃 https://bundlephobia.com/result?p=@primer/components@0.0.0-acfdf18 Curious what you think @BinaryMuse

All of that cut our bundle size almost in half 馃帀 It's now sitting at 49kb, down from 84.8kb.

Sidenote: If we move over to @github/details-dialog-element for our Dialog I think we can get down to ~ 28kb. Still feels semi-big but not sure what to use as a benchmark.

image
byeeeee! 馃憢馃憢馃憢

@vercel
Copy link

vercel bot commented Oct 24, 2019

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

馃攳 Inspect: https://zeit.co/primer/primer-components/ea8ehz1e4
馃實 Preview: https://primer-components-git-cleanup-deps.primer.now.sh

@vercel vercel bot temporarily deployed to staging October 24, 2019 19:22 Inactive
@vercel vercel bot temporarily deployed to staging October 24, 2019 19:25 Inactive
@vercel vercel bot temporarily deployed to staging October 24, 2019 19:27 Inactive
@vercel vercel bot temporarily deployed to staging October 24, 2019 19:37 Inactive
@vercel vercel bot temporarily deployed to staging October 24, 2019 19:41 Inactive
@vercel vercel bot temporarily deployed to staging October 24, 2019 19:56 Inactive
@vercel vercel bot temporarily deployed to staging October 24, 2019 20:00 Inactive
@vercel vercel bot requested a deployment to staging October 24, 2019 20:07 Abandoned
@vercel vercel bot temporarily deployed to staging October 24, 2019 20:07 Inactive
@vercel vercel bot temporarily deployed to staging October 24, 2019 20:07 Inactive
@vercel vercel bot temporarily deployed to staging October 24, 2019 20:16 Inactive
@BinaryMuse
Copy link
Contributor

but for some reason bundlephobia classifies it as a missing dependency

Weird, I'm not sure either. At first I thought it was maybe a peerDep of something? But no. Maybe bundlephobia assumes it exists when it sees React?

@emplums emplums merged commit a80c8c6 into release-15.1.0 Nov 4, 2019
@emplums emplums deleted the cleanup-deps branch November 4, 2019 22:43
@emplums emplums mentioned this pull request Nov 4, 2019
10 tasks
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.

None yet

2 participants