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

v5.2.0 tree shaking doesn't work as intended #140

Closed
TommasoAmici opened this issue Aug 8, 2022 · 18 comments
Closed

v5.2.0 tree shaking doesn't work as intended #140

TommasoAmici opened this issue Aug 8, 2022 · 18 comments

Comments

@TommasoAmici
Copy link
Contributor

I've been using this library for a long time and I'm very thankful for your work!

I've recently updated to v5.2.0 and tree shaking is now broken. I saw this issue #32 but I guess something must've changed since v3.

This is how I'm importing the icons:

import { Facebook, Instagram, Twitter } from "@icons-pack/react-simple-icons";

It's a Next.js application with Typescript. Below you can see a before and after in bundle size:

No icons:

Page                                       Size     First Load JS
┌ ● /                                      811 B           129 kB
...

With icons:

Page                                       Size     First Load JS
┌ ● /                                      811 B          1.68 MB
...
@TommasoAmici
Copy link
Contributor Author

I have now downgraded to v4.7.2 and tree shaking works properly. I'm guessing something is not configured correctly in the new Rollup config in this commit 362600c

@wootsbot
Copy link
Member

Fixed in v5.3.0

@TommasoAmici
Copy link
Contributor Author

Awesome, thanks for making a release!

@sch-28
Copy link

sch-28 commented Nov 23, 2022

This issue seems to be back since v.6.0.0

@TommasoAmici
Copy link
Contributor Author

It is :/

@wootsbot
Copy link
Member

@sch-28 Can you show the results with the latest version?

@skyclo
Copy link

skyclo commented Jan 8, 2023

Is simple-icons/simple-icons#8112 possibly related? I can confirm that this is an issue for me and prevents my Remix app from deploying on CF Pages (bundle size is > 1MB when using a few icons).

Edit: Downgrading to 5.11 fixes the issue.

@aflatter
Copy link

Edit: Downgrading to 5.11 fixes the issue.

Thanks, I just downgraded to 5.30 and it works for me.

@wootsbot wootsbot reopened this Apr 3, 2023
@wootsbot
Copy link
Member

wootsbot commented Apr 3, 2023

If I have suffered my own mistake, working on it.

@wootsbot
Copy link
Member

wootsbot commented Apr 4, 2023

@aflatter @skyclo I am checking with version v7.1.1 I have a tree correctly

import {
  SiReact,
  Si1001tracklists,
  SiAbbott,
  Si1password,
} from "@icons-pack/react-simple-icons";

function IconsPage() {
  return (
    <div>
      <SiReact color="#61DAFB" size={24} />
      <Si1001tracklists color="#61DAFB" size={24} />
      <SiAbbott color="#61DAFB" size={24} />
      <Si1password color="#61DAFB" size={24} />
    </div>
  );
}

export default IconsPage;

CleanShot 2023-04-03 at 22 17 18@2x

@aflatter
Copy link

aflatter commented Apr 6, 2023

@aflatter @skyclo I am checking with version v7.1.1 I have a tree correctly

Just tried 7.1.1, same problem:

foo@bar aflatter % npm run build

> aflatter@0.1.0 build
> next build

info  - Linting and checking validity of types  
info  - Compiled successfully
info  - Collecting page data  
info  - Generating static pages (4/4)
info  - Finalizing page optimization  

Route (pages)                              Size     First Load JS
┌ ○ /                                      1.63 MB        1.71 MB
├   /_app                                  0 B            73.3 kB
├ ○ /404                                   182 B          73.4 kB
├ ○ /about                                 1.6 kB         80.1 kB
└ λ /api/hello                             0 B            73.3 kB
+ First Load JS shared by all              75.6 kB
  ├ chunks/framework-2c79e2a64abdb08b.js   45.2 kB
  ├ chunks/main-0ecb9ccfcb6c9b24.js        27 kB
  ├ chunks/pages/_app-ae907860a06fe57a.js  296 B
  ├ chunks/webpack-8fa1640cc84ba8fe.js     750 B
  └ css/5983c06de8f68d75.css               2.37 kB


foo@bar aflatter % git diff package.json
diff --git a/package.json b/package.json
index 572584b..78a7a57 100644
--- a/package.json
+++ b/package.json
@@ -9,21 +9,21 @@
     "lint": "next lint"
   },
   "dependencies": {
-    "@icons-pack/react-simple-icons": "^5.3.0",
+    "@icons-pack/react-simple-icons": "^7.1.1",
     "@types/node": "18.14.6",
     "@types/react": "18.0.28",
     "@types/react-dom": "18.0.11",
     "eslint": "8.35.0",
     "eslint-config-next": "13.2.3",
-    "next": "13.2.3",
+    "next": "^13.2.4",
     "react": "18.2.0",
     "react-dom": "18.2.0",
     "sharp": "^0.31.3",
     "typescript": "4.9.5"
   },
   "devDependencies": {
-    "autoprefixer": "^10.4.13",
+    "autoprefixer": "^10.4.14",
     "postcss": "^8.4.21",
-    "tailwindcss": "^3.2.7"
+    "tailwindcss": "^3.3.1"
   }
 }

@wootsbot
Copy link
Member

wootsbot commented Apr 7, 2023

Just tried 7.1.1, same problem:

Let me replicate your settings, I'll do some deeper tests

@AbdBarho
Copy link

We are also facing the same problem:

https://github.com/LAION-AI/Open-Assistant/tree/main/website

"@icons-pack/react-simple-icons": "^7.2.0"

@wootsbot
Copy link
Member

@AbdBarho I'm working on a compiled refactor, I think at the moment it can help to solve

#140 (comment)

@AbdBarho
Copy link

thanks @wootsbot, it solved the situation.

I was wondering, would it be a possible to import icons individually?

import SiGithub from "@icons-pack/react-simple-icons/Github" //or something

This has the advantage of being very explicit, while also giving faster build times because the bundler does not have to tree shake the entire package in every file where an icon is imported (looking at you MUI).

AbdBarho added a commit to LAION-AI/Open-Assistant that referenced this issue Apr 15, 2023
The latest version has a tree shaking problem
icons-pack/react-simple-icons#140, which
caused some of our pages to have 2MB! of javascript:


https://github.com/LAION-AI/Open-Assistant/actions/runs/4706456312/jobs/8347722304#step:8:449

The problem does not exists in older versions, so I downgraded until it
is fixed.
@ruchernchong
Copy link

@AbdBarho I'm working on a compiled refactor, I think at the moment it can help to solve

#140 (comment)

This works for now. Hoping to see a proper resolution for the v7s soon

@wootsbot
Copy link
Member

This has been resolved in version v8.0.1. I have had the opportunity to submit this refactoring. Please, feel free to report if you encounter any issues.

@wootsbot
Copy link
Member

@AbdBarho I'm working on a compiled refactor, I think at the moment it can help to solve
#140 (comment)

This works for now. Hoping to see a proper resolution for the v7s soon

This has been resolved in version v8.0.1.

ruchernchong added a commit to ruchernchong/portfolio that referenced this issue Jun 15, 2023
Fix huge package issue when build: icons-pack/react-simple-icons#140
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

No branches or pull requests

7 participants