Fix tree-shaking issues

[mbrowne]

This is a long PR description, but it's a complex issue so hopefully the explanation is helpful...

This PR resolves #245, and also another issue I found when calling helper functions/template literals (more on that below).

Fixing tree-shaking when static properties are used

As a solution for #245, this PR implements the following:

• Styled components with static properties (example) will be wrapped with an IIFE and a PURE comment will be added to the IIFE
• Function components that consume styled components (i.e. will or might render them) (example) will also be wrapped with an IIFE with a PURE comment. Technically this is only necessary in some cases, and I didn't investigate exactly what triggers it. But given that the issue happens even for relatively simple components like this one, I figured it was reasonable to go ahead and wrap all function components that consume styled components with IIFEs.

This PR does not address class components; I have found that class components often still have issues with tree-shaking even after applying these kinds of fixes. By coincidence, this PR can help with class components too if transpiling to ES5, because then the classes get turned into functions and my function-component detection code then thinks that they're function components. So I don't think it's worth the effort to apply transformations to class components directly. (By the time people are transpiling to ES6 more regularly, they'll probably be phasing out many of their class components anyway, or at least not writing new ones.)

Fixing tree-shaking when inline calculations are used in template expressions

Here is an example of this issue:
https://github.com/mbrowne/CRA-error-template/blob/styled-components-tree-shaking/src/components/Component5.js

This one seems to be an issue with terser's tree-shaking algorithm.

If you're just using a helper or template literal inside a template expression passed to styled.div or any HTML element, it seems that tree-shaking works fine. But when using styled(CustomComponent), there are issues (which oddly can cause CustomComponent to be included in the bundle, but not necessarily the component extending it). I wasn't sure exactly what triggers these issues either, so I decided to just add a PURE comment to any call to a helper function or template literal.

Remaining issues

Native template literals

The most significant remaining issue I found is the case where the user sets pure to true but transpileTemplateLiterals to false. When targeting ES6 (e.g. easiest way to test it is to remove the @babel/preset-env preset), even a very basic component like this will fail to tree-shake:

export const Component = styled.div`
  color: turquoise;
`

Tree-shaking will still work correctly if targeting ES5, because then the template literals get transpiled to functions anyhow, but if targeting only browsers supporting ES6 (which I'm sure will become more common in the future), that would theoretically be a good reason to set transpileTemplateLiterals to false. Unfortunately, it seems that native template literals break tree-shaking in both terser and rollup. I'm not sure what can be done about this, since adding a PURE comment in front of the template literals had no effect. My guess is that because it's a tagged template literal, terser/rollup doesn't know whether or not there might be side-effects. Probably the best solution would be for terser and rollup to add support for PURE comments on tagged template literals, which apparently are not currently supported.

Definitely something to keep an eye on in the future.

React.forwardRef

React.forwardRef breaks tree-shaking, but that happens all the time and not just for styled-components. It should probably be addressed by adding a new plugin to @babel/preset-react to address this. So it's kind of off-topic, but I figured I would mention it since it could be a reason why a component consuming styled components isn't tree-shaking.

---

Testing Tip

A tip on testing this in create-react-app, or any app using webpack and babel-loader: babel-loader caches Babel plugins in node_modules/.cache. So if you're manually copying the new version of the plugin over to test it (like I was doing), make sure to delete that cache folder first (I was rather confused by this until I realized it was caching ;) ).

[donifer]

Any updates on this? Also is there a workaround for React.forwardRef not to break 🌳shaking? Can I mark it as PURE or something?

[quantizor]

Also is there a workaround for React.forwardRef not to break 🌳shaking?

forwardRef comes from the main library so perhaps some pure comments should be added in StyledComponent.js etc.

[deini]

Thanks for doing this @mbrowne 🙇

Just tested this against @bigcommerce/big-design and seems to be working as expected.

[mbrowne]

Great, thanks for testing it out @deini!

Regarding React.forwardRef, while it would be possible to add pure comments to that as well, I think it would make more sense to create a new Babel plugin for that since it affects anyone who uses forwardRef, regardless of whether or not they're using styled-components. It hasn't been a priority for me to work on that, but my plan was to create that Babel plugin and then submit a PR for @babel/preset-react (which is part of the main Babel repo) so it would be included for everyone by default.

In the meantime, while it's possible to add your own PURE comments, note that you'll need to adjust your Babel settings because ordinarily all comments (including PURE comments) are removed during transpilation. Between that and the fact that always having to manually write your own PURE comments is annoying, a Babel plugin is definitely the preferred way to add PURE annotations.

However, there is another way of handling this (until I or someone writes that Babel plugin for React.forwardRef) that you might find easier: terser has an option to tell it that certain functions are pure functions and it can ignore them when it's trying to remove dead code (i.e. tree-shaking). For example, if you were using rollup-plugin-terser, you could do this:

plugins: [
    terser({
        compress: {
            pure_funcs: ['React.forwardRef']
        }
    })
]

[donifer]

Adding it to a new Babel plugin or @babel/preset-react sound like the right approach (and not here). I believe React.memo has the same issue and also would be great to have it annotated.

[mryechkin]

Hi @mbrowne - have there been any updates on this? I came across your solution after countless hours of trying to figure out tree-shaking issues in a component library I'm working on, which ultimately led me to your babel-plugin-pure-static-props and then here. Would love to be able to use this, but frankly not sure how to proceed. Any help would be greatly appreciated!

[mbrowne]

@mryechkin Unfortunately this PR never got much more attention. I would love to see some follow-up on this, and I think it would also be good for more people to test it. But if you're looking for a solution for production, my earlier babel-plugin-pure-static-props plugin is probably the best option for now. It should still work even though I said it's deprecated in the readme. (Run npm run build, and see the demo folder for example usage.)

[mryechkin]

@mbrowne thanks for the quick reply! Just tried it out, and the demo is working as expected however when I add styled-components and include displayName, that component doesn't seem to get annotated as pure.

[redacted]

Any ideas on this? Thanks again, much appreciated

EDIT: Please disregard, I just realized that the earlier plugin doesn't have that functionality, and hence this PR 🤦🏻‍♂️

[mbrowne]

@mryechkin FYI, it should in theory be possible to copy over some of the styled-components detection logic from this plugin over to your own plugin based on my babel-plugin-pure-static-props, e.g. this function:
https://github.com/styled-components/babel-plugin-styled-components/blob/master/src/utils/detectors.js#L66

But it might actually be easier to just use a forked version of this plugin using my PR (surprisingly there seem to be no merge conflicts with the latest master in the actual code, so you'd still be getting all the other latest features). One thing I've found helpful in the past to create an installable npm package without adding it to the public registry (or setting up a private one), is to use this tool:
https://www.npmjs.com/package/gitpkg

[mryechkin]

@mbrowne I think I may have gotten it to work without having to modify anything!

I used both the plugin from this PR and your babel-plugin-pure-static-props together in my Rollup config, which looks to have done the trick 🙌🏻

Also I haven't tried merging this with the latest master yet (precisely because I figured there would be conflicts) - so that's great to hear.

As you suggest, we will likely end up maintaining our own fork, to be used by the component library. Not ideal, of course - but I'm just happy to get it working to begin with!

Thanks very much for your help, really appreciate it. Hopefully this gets picked up soon!

[mbrowne]

Glad to hear you got it working! babel-plugin-pure-static-props shouldn't be needed if you're already using the forked plugin based on this PR. If it doesn't work anymore after removing babel-plugin-pure-static-props, then that means I have more work to do on this PR...unless you were also encountering tree-shaking issues with something unrelated to styled-components that's using static class properties in a similar way.

[mryechkin]

unless you were also encountering tree-shaking issues with something unrelated to styled-components that's using static class properties in a similar way

@mbrowne yes that's exactly it - without babel-plugin-pure-static-props regular function components with displayName weren't annotated. As far styled-components go this PR version is working as expected without it 👍