'PURE' Object.assign statement gets incorrectly removed

[mbrowne]

• Rollup Version: 1.20.3
• Operating System (or Browser): OSX
• Node Version: 10.16.0

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:

function MyComponent() {}
/*#__PURE__*/
Object.assign(MyComponent, { displayName: 'Foo' })

export { MyComponent }

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 or defaultProps. 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, the Object.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.

[TrySound]

You should not mark Object.assign as pure. It's side effect. You should assign property explicitly to function component to treeshake module correctly.
https://rollupjs.org/repl/?version=1.20.3&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmZ1bmN0aW9uJTIwTXlDb21wb25lbnQoKSUyMCU3QiU3RCU1Q25NeUNvbXBvbmVudC5kaXNwbGF5TmFtZSUyMCUzRCUyMCdGb28nJTIyJTJDJTIyaXNFbnRyeSUyMiUzQXRydWUlN0QlNUQlMkMlMjJvcHRpb25zJTIyJTNBJTdCJTIyZm9ybWF0JTIyJTNBJTIyY2pzJTIyJTJDJTIybmFtZSUyMiUzQSUyMm15QnVuZGxlJTIyJTJDJTIyYW1kJTIyJTNBJTdCJTIyaWQlMjIlM0ElMjIlMjIlN0QlMkMlMjJnbG9iYWxzJTIyJTNBJTdCJTdEJTdEJTJDJTIyZXhhbXBsZSUyMiUzQW51bGwlN0Q=

[lukastaegert]

To achieve something closer to what you want, instead of using Object.assign as a side-effectful statement but marking it as side-effect-free, you could use the return value of Object.assign as your component. Then the PURE annotation actually makes sense as the side-effect is only relevant if the component is used:

https://rollupjs.org/repl/?version=1.20.3&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmltcG9ydCUyMCU3QiUyMENvbXBvbmVudDElMjAlN0QlMjBmcm9tJTIwJy4lMkZDb21wb25lbnQxLmpzJyUzQiU1Q25pbXBvcnQlMjAlN0IlMjBDb21wb25lbnQyJTIwJTdEJTIwZnJvbSUyMCcuJTJGQ29tcG9uZW50Mi5qcyclM0IlNUNuJTVDbiUyRiUyRiUyME9ubHklMjB1c2UlMjBvbmUlMjBvZiUyMHRoZSUyMGNvbXBvbmVudHMlNUNuY29uc29sZS5sb2coQ29tcG9uZW50MSklM0IlMjIlMkMlMjJpc0VudHJ5JTIyJTNBdHJ1ZSU3RCUyQyU3QiUyMm5hbWUlMjIlM0ElMjJDb21wb25lbnQxLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmNvbnN0JTIwQ29tcG9uZW50MSUyMCUzRCUyMCUyRiolMjNfX1BVUkVfXyolMkYlMjBPYmplY3QuYXNzaWduKCUyMGZ1bmN0aW9uJTIwQ29tcG9uZW50MSgpJTIwJTdCJTdEJTJDJTIwJTdCJTIwZGlzcGxheU5hbWUlM0ElMjAnRm9vJyUyMCU3RCklNUNuJTVDbmV4cG9ydCUyMCU3QiUyMENvbXBvbmVudDElMjAlN0QlMjIlN0QlMkMlN0IlMjJuYW1lJTIyJTNBJTIyQ29tcG9uZW50Mi5qcyUyMiUyQyUyMmNvZGUlMjIlM0ElMjJjb25zdCUyMENvbXBvbmVudDIlMjAlM0QlMjAlMkYqJTIzX19QVVJFX18qJTJGJTIwT2JqZWN0LmFzc2lnbiglMjBmdW5jdGlvbiUyMENvbXBvbmVudDIoKSUyMCU3QiU3RCUyQyUyMCU3QiUyMGRpc3BsYXlOYW1lJTNBJTIwJ0ZvbyclMjAlN0QpJTVDbiU1Q25leHBvcnQlMjAlN0IlMjBDb21wb25lbnQyJTIwJTdEJTIyJTdEJTVEJTJDJTIyb3B0aW9ucyUyMiUzQSU3QiUyMmZvcm1hdCUyMiUzQSUyMmVzbSUyMiUyQyUyMm5hbWUlMjIlM0ElMjJteUJ1bmRsZSUyMiUyQyUyMmFtZCUyMiUzQSU3QiUyMmlkJTIyJTNBJTIyJTIyJTdEJTJDJTIyZ2xvYmFscyUyMiUzQSU3QiU3RCU3RCUyQyUyMmV4YW1wbGUlMjIlM0FudWxsJTdE

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 PURE that returns the finished object:

https://rollupjs.org/repl/?version=1.20.3&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmltcG9ydCUyMCU3QiUyMENvbXBvbmVudDElMjAlN0QlMjBmcm9tJTIwJy4lMkZDb21wb25lbnQxLmpzJyUzQiU1Q25pbXBvcnQlMjAlN0IlMjBDb21wb25lbnQyJTIwJTdEJTIwZnJvbSUyMCcuJTJGQ29tcG9uZW50Mi5qcyclM0IlNUNuJTVDbiUyRiUyRiUyME9ubHklMjB1c2UlMjBvbmUlMjBvZiUyMHRoZSUyMGNvbXBvbmVudHMlNUNuY29uc29sZS5sb2coQ29tcG9uZW50MSklM0IlMjIlMkMlMjJpc0VudHJ5JTIyJTNBdHJ1ZSU3RCUyQyU3QiUyMm5hbWUlMjIlM0ElMjJDb21wb25lbnQxLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmNvbnN0JTIwQ29tcG9uZW50MSUyMCUzRCUyMCUyRiolMjNfX1BVUkVfXyolMkYlMjAoZnVuY3Rpb24lMjAoKSUyMCU3QiU1Q24lMjAlMjBmdW5jdGlvbiUyMENvbXBvbmVudDEoKSUyMCU3QiU3RCU1Q24lNUN0Q29tcG9uZW50MS5kaXNwbGF5TmFtZSUyMCUzRCUyMCdmb28nJTNCJTVDbiU1Q3RyZXR1cm4lMjBDb21wb25lbnQxJTNCJTVDbiU3RCUyMCgpKSU1Q24lNUNuZXhwb3J0JTIwJTdCJTIwQ29tcG9uZW50MSUyMCU3RCUyMiU3RCUyQyU3QiUyMm5hbWUlMjIlM0ElMjJDb21wb25lbnQyLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmNvbnN0JTIwQ29tcG9uZW50MiUyMCUzRCUyMCUyRiolMjNfX1BVUkVfXyolMkYlMjAoZnVuY3Rpb24lMjAoKSUyMCU3QiU1Q24lMjAlMjBmdW5jdGlvbiUyMENvbXBvbmVudDIoKSUyMCU3QiU3RCU1Q24lNUN0Q29tcG9uZW50Mi5kaXNwbGF5TmFtZSUyMCUzRCUyMCdmb28nJTNCJTVDbiU1Q3RyZXR1cm4lMjBDb21wb25lbnQyJTNCJTVDbiU3RCUyMCgpKSU1Q24lNUNuZXhwb3J0JTIwJTdCJTIwQ29tcG9uZW50MiUyMCU3RCUyMiU3RCU1RCUyQyUyMm9wdGlvbnMlMjIlM0ElN0IlMjJmb3JtYXQlMjIlM0ElMjJlc20lMjIlMkMlMjJuYW1lJTIyJTNBJTIybXlCdW5kbGUlMjIlMkMlMjJhbWQlMjIlM0ElN0IlMjJpZCUyMiUzQSUyMiUyMiU3RCUyQyUyMmdsb2JhbHMlMjIlM0ElN0IlN0QlN0QlMkMlMjJleGFtcGxlJTIyJTNBbnVsbCU3RA==

[mbrowne]

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 Object.assign works (to prevent rollup from removing it), but requires that you use a different variable name for the React component prior to calling Object.assign, or that you declare the function inline as you (@lukastaegert) did in your example. So I'll probably go with the IIFE option, which would eliminate the need to use Object.assign in the first place—I can just use MyComponent.displayName = 'Foo' inside the IIFE. Now I just need to update my Babel plugin to do this, which makes it a little more complicated but I'm sure it's doable.

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.

[mbrowne]

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 Object.assign statement (just like rollup) if the return value is unused and the Object.assign call is annotated with /*#__PURE__*/. So the bottom line is to always use the return value, because otherwise the pure annotation is telling it that it's a useless statement (i.e. has no effect on the functioning of the code) that can be removed.