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

String(Symbol()) is removed upon treeshaking #2790

Closed
alippai opened this issue Apr 3, 2019 · 4 comments
Closed

String(Symbol()) is removed upon treeshaking #2790

alippai opened this issue Apr 3, 2019 · 4 comments

Comments

@alippai
Copy link
Contributor

alippai commented Apr 3, 2019

  • Rollup Version:
  • Operating System (or Browser):
  • Node Version:

How Do We Reproduce?

  1. Add core-js@3.0.0
  2. Use rollup
  3. error in IE11 with "internals/well-known-symbol.js" zloirock/core-js#513 happens

Expected Behavior

String(Symbol()) shouldn't be removed as they can easily throw

Actual Behavior

They are removed

@lukastaegert
Copy link
Member

as they can easily throw

not to my knowledge in an environment which is compliant with the current ES2018 spec. Which is not to dismiss this issue but to put it into perspective. Rollup currently assumes that all builtins behave according to the latest spec.

And this is usually what the user wants EXCEPT inside polyfills. If every invocation of Symbol would be considered a side-effect, this would make Symbol something that people should definitely avoid ever using unless forced to.

So this is more about the issue of providing a useful fallback behaviour for polyfills (BTW all of these issues stem from IE11...). I have several ideas but nothing has been investigated further yet.

@alippai
Copy link
Contributor Author

alippai commented Apr 3, 2019

Wow, thank you for sharing the context of the issue!

This specific incompatibility between Rollup and core-js will be fixed upon the next core-js release.

Idea

Looking for a workaround myself (without patching core-js or Rollup) I was expecting a blacklist option for the tree-shaking algorithm, where I can flag functions (names) with known side-effects (in this case Symbol()). I know this wouldn't be an optimal solution either, but maybe it even helps with propertyReadSideEffects (which could default to false).
// @rollup-side-effect-next-line is an other option (which wouldn't help in my case)

My solution

To resolve my issue I didn't bundle core-js, but added it separately to the HTML. It's a separate vendor package with separate release cycle, already bundled and it's huge alone (30+kB).

@GrosSacASac
Copy link

related #1771

nicketson added a commit to turbobridge/core-js that referenced this issue Apr 25, 2019
rollup treeshaking is removing the Object.getOwnPropertyNames
FAILS_ON_PRIMITIVES feature test.

This is related to these rollup issue
rollup/rollup#1771
rollup/rollup#2790

Related issue and fix
zloirock#513
zloirock@f813c4e
nicketson added a commit to turbobridge/core-js that referenced this issue Apr 26, 2019
rollup treeshaking is removing the Object.getOwnPropertyNames
FAILS_ON_PRIMITIVES feature test.

This is related to these rollup issue
rollup/rollup#1771
rollup/rollup#2790

Related issues
zloirock#494
zloirock#513
@lukastaegert
Copy link
Member

This should be solved via #2892. If not, feel free to reopen and explain.

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

3 participants