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

Tree shaking #478

Open
sayjeyhi opened this issue Feb 29, 2020 · 21 comments
Open

Tree shaking #478

sayjeyhi opened this issue Feb 29, 2020 · 21 comments

Comments

@sayjeyhi
Copy link

sayjeyhi commented Feb 29, 2020

Hi there,

I was analyzing my packages and figure out that all of polished function did imported and impact my bundle size.
I just used darken and lighten functions.
here is bundlephobia which tell all single function has same size as all package :
https://bundlephobia.com/[email protected]
see Exports Analysis part

@bhough bhough added the question label Mar 1, 2020
@bhough
Copy link
Contributor

bhough commented Mar 1, 2020

@sayjeyhi polished is fully tree-shakeable and has been for a couple years now. FWIW I don't think Bundlephobia is evidence that something isn't properly tree-shakeable. Here is another example where it is incorrect with styled-components: https://bundlephobia.com/[email protected].

We will need more information about your setup to provide any help on why it isn't working in your particular instance:

  • Version of polished you are using.
  • Example of how you are using/importing the functions.
  • Which bundler and version you are using.
  • Config for said bundler.

Our docs also link to how to set this up in webpack and rollup: https://github.com/styled-components/polished#usage

@sayjeyhi
Copy link
Author

sayjeyhi commented Mar 4, 2020

so you say this is just for lighten and darken functions :
Screen Shot 1398-12-14 at 11 29 37 PM

I am using webpack 4.41.2 , and I did import polished in es module way(import {foo} from 'bar') and my polished version is 3.4.2

@sayjeyhi
Copy link
Author

sayjeyhi commented Mar 5, 2020

Screen Shot 1398-12-15 at 10 14 21 AM

I import lighten by adding full path and bundle size gets better result.

import lighten from 'polished/lib/color/lighten';

@avin-kavish
Copy link

avin-kavish commented Apr 19, 2020

@bhough I'm aware of sub-module imports. But we can't really classify that as a solution can we? Tree shaking is designed to work on re-exported modules at the root level. There has to be a reason for it to break only on this library.

@bhough
Copy link
Contributor

bhough commented Apr 19, 2020

@avin-kavish This is still an open issue @sayjeyhi is just suggesting an interim solution. We've been unable to recreate this yet. If you'd like to provide the info we asked the OP for, we can continue to dig into it.

One thing that usually catches people is they are running their bundle analyzer on a development build vs the production bundle. In development, you will see the entire library. When you use the method described above, you will see just this file imported into the development build directly, which is not the same thing as the tree-shaking going on in the production build.

@sayjeyhi
Copy link
Author

@bhough maybe I could work around it this weekend and come with a PR...

@bhough
Copy link
Contributor

bhough commented Apr 19, 2020

@sayjeyhi All we really need is a link to a repo we can fork that shows where it isn't tree-shaking and can take it from there.

@bhough
Copy link
Contributor

bhough commented Apr 19, 2020

image

@sayjeyhi the screenshot is lighten and darken using a basic CRA app out of the box. The first difference I see is your build is pulling from lib where mine is pulling from dist, so that is likely a good place for us to start.

@avin-kavish
Copy link

avin-kavish commented Apr 20, 2020

Yeah, it works with minimal code, trouble happens when there's a complex tree of dependencies.

polished
|-grommet (another npm package)
   |-Layout A
   |  |-Page A
   |-Layout B
      |-Page B

So I created a tree like this in Next.js (which is where I am having the trouble) but I can't get an accurate size because of module concatenation. I will keep experimenting...

Edit: With module concat turned off, it's only adding 2.5kB so no go once again.

@bhough
Copy link
Contributor

bhough commented Apr 20, 2020

Edit: With module concat turned off, it's only adding 2.5kB so no go once again.

Not sure what you mean by 'no go once again'? The fact that it is only adding 2.5kB should mean it is working correctly. Keep in mind color modules are relatively heavy given their dependencies.

@avin-kavish
Copy link

avin-kavish commented Apr 20, 2020

I was trying to create a reproduction of the issue in a code base separate from the one I'm having the issue in. I can't share the original as it is not open source. What I mean is that the issue is likely due to some weird interaction between dependencies etc in much larger/complex code.

@bhough
Copy link
Contributor

bhough commented Apr 20, 2020

K @avin-kavish We will keep this up for the time being to see if we can get a recreation. Only issue we've seen is folks pulling in the non-es-module version from the main field and thus not getting tree-shaking. We are going to do a bit of work to make that less impactful when it happens.

@avin-kavish
Copy link

avin-kavish commented Apr 20, 2020

I see. I don't think that's happened in my case based on the screenshot below. It seems that polished.es.js is imported. Maybe the following info could be helpful?

yarn why polished

=> Found "[email protected]"
info Reasons this module exists
   - "grommet" depends on it
   - Hoisted from "grommet#polished"
info Disk size without dependencies: "3.49MB"
info Disk size with unique dependencies: "4.17MB"
info Disk size with transitive dependencies: "4.21MB"
info Number of shared dependencies: 2

Bundle Analyzer

Full Bundle with Polished: 432kb.
Full Bundle without Polished: 422kb.

How did I remove only polished if my project depends on grommet, a 3rd party library which depends on polished?

I created my own fork of grommet and removed all references to polished. Luckily, on the rgba function was imported 3 times in the whole project. No other functions were imported. Their use case was trivial enough to replace with strings. In one situation, I broke a certain component, but since I don't use that component in my project, it's no issue for me.

@bhough
Copy link
Contributor

bhough commented Apr 20, 2020

@avin-kavish That's good context. The fact that it is being pulled in from another dep could be significant. Could be something grommet is doing in their implementation that keeps it from being tree-shaken. Will explore this further.

@avin-kavish
Copy link

avin-kavish commented Apr 22, 2020

Could it be this?

exports.__esModule = true;

var _components = require("./components");

Object.keys(_components).forEach(function (key) {
  if (key === "default" || key === "__esModule") return;
  exports[key] = _components[key];
});

Their exports seems to be dynamically defined.

@twdrake-ap
Copy link

I'm running into this issue as well, import { darken, lighten } from 'polished'; is causing the whole package to be bundled with a basic webpack config. I'm on version 3.6.6.

@langri-sha
Copy link

☝️ same issue as @twdrake-ap. On a basic Gatsby project, using import { transparentize } from 'polished, I get 100 modules concatenated together with the main package module.

@Armanio
Copy link

Armanio commented Feb 25, 2021

Same issue. 🤔
If using direct import with full path works fine.

Simple workaround.
before:
Снимок экрана 2021-02-25 в 18 52 56

after:
Снимок экрана 2021-02-25 в 18 51 34

@hipstersmoothie
Copy link

Wild shot in the dark: Everyone in this thread is including source maps it some way

With devtool: 'source-map'

CleanShot 2021-10-11 at 13 54 21

With devtool: false

CleanShot 2021-10-11 at 13 54 02

@sbdchd
Copy link

sbdchd commented Jan 1, 2022

@hipstersmoothie disabling source-maps is a no go for me because they're needed for debugging errors in production

@Profesor08
Copy link

export default in every module is causing this issue when you import some function with import { fn } from "polished";. Avoiding of using default exports will solve this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants