-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
'PURE' Object.assign statement gets incorrectly removed #3085
Comments
You should not mark Object.assign as pure. It's side effect. You should assign property explicitly to function component to treeshake module correctly. |
To achieve something closer to what you want, instead of using Another common technique to hide mutations of an object in a way that tree-shaking works is to wrap the problematic code in an IIFE annotated as |
Thanks for the tips. I think I understand now why it's removing it, and it makes sense. Interestingly, uglify-js doesn't remove it, and I thought that the rollup implementation was based on the same semantics as uglify. But uglify isn't trying to do tree-shaking per se, just dead code elimination. Using the return value of In summary: I'm going to assume that although rollup's behavior is different from what uglify does in this case, it's working as intended and is not a bug, so I'm closing this issue. |
Never mind about uglify-js—I thought it removed dead code by default, so when I was testing my assumptions earlier by running uglify-js directly on the command line, that test wasn't valid. With dead code elimination enabled, uglify-js does remove the |
How Do We Reproduce?
https://rollupjs.org/repl/?version=1.20.3&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmZ1bmN0aW9uJTIwTXlDb21wb25lbnQoKSUyMCU3QiU3RCU1Q24lMkYqJTIzX19QVVJFX18qJTJGJTVDbk9iamVjdC5hc3NpZ24oTXlDb21wb25lbnQlMkMlMjAlN0IlMjBkaXNwbGF5TmFtZSUzQSUyMCdGb28nJTIwJTdEKSU1Q24lNUNuZXhwb3J0JTIwJTdCJTIwTXlDb21wb25lbnQlMjAlN0QlMjIlMkMlMjJpc0VudHJ5JTIyJTNBdHJ1ZSU3RCU1RCUyQyUyMm9wdGlvbnMlMjIlM0ElN0IlMjJmb3JtYXQlMjIlM0ElMjJjanMlMjIlMkMlMjJuYW1lJTIyJTNBJTIybXlCdW5kbGUlMjIlMkMlMjJhbWQlMjIlM0ElN0IlMjJpZCUyMiUzQSUyMiUyMiU3RCUyQyUyMmdsb2JhbHMlMjIlM0ElN0IlN0QlN0QlMkMlMjJleGFtcGxlJTIyJTNBbnVsbCU3RA==
Expected Behavior
I expect the
Object.assign()
statement to be included in the output given this input:Actual Behavior
The
Object.assign
call is completely removed.Details
I am trying to get tree-shaking to work correctly with React components that use styled-components and also have static properties like
displayName
ordefaultProps
. Unfortunately, the presence of any such static properties causes uglify-js to include the whole component in the bundle, even if it's never imported anywhere—see styled-components/babel-plugin-styled-components#245.My workaround is to use
Object.assign
instead of setting the static properties directly, so that I can annotate it with a/*#__PURE__*/
comment. But because these React components are in a library that I'm building with rollup, theObject.assign
calls get completely removed.For now, the only way I'm able to get around this is by turning off
treeshake.annotations
in the rollup config, but I'd prefer to leave it enabled. And of course this seems like a bug that should be fixed in any case.The text was updated successfully, but these errors were encountered: