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

bug with side_effects option #3842

Closed
LongTengDao opened this issue May 3, 2020 · 6 comments
Closed

bug with side_effects option #3842

LongTengDao opened this issue May 3, 2020 · 6 comments

Comments

@LongTengDao
Copy link

LongTengDao commented May 3, 2020

Bug report or feature request? Bug report

Uglify version (uglifyjs -V) 3.9.2

JavaScript input

function f () {
/*#__PURE__*/a();
/*#__PURE__*/b();
}

The uglifyjs CLI command executed or minify() options used.

( {
	compress: {
		side_effects: false,
	},
	output: {
		comments: function () { return true; },
	},
} )

JavaScript output or error produced.

function f(){
/* */a()}

There are multi problems in the sample.

  1. b() disapeared
  2. I can't keep /*#__PURE__*/ even when set side_effects to false, to leave it open to downstream user to make the decision. Currently, I must decide, whether to treeshake the pure calling, or miss the comment and it will never be treeshaked.

Expected output

function f(){/*#__PURE__*/a(),/*#__PURE__*/b()}
@alexlamsl
Copy link
Collaborator

There are no bugs:

@LongTengDao
Copy link
Author

@alexlamsl When side_effects is false, the call after /*#__PURE__*/ should not be treated as pure call as doc said, so I can't understand, why there is no bug while non-pure b() is deleted?

@LongTengDao
Copy link
Author

And I can't understand the source in fact... Do you mean directly remove the line 2274?

@alexlamsl
Copy link
Collaborator

No, I mean something like:

if (compressor.option("side_effects") && seq.length > 0) {
    body = body.drop_side_effect_free(compressor);
}

@LongTengDao
Copy link
Author

@fabiosantoscode Hi~ Would you help to fix this? Maybe the same problem in terser will be fixed together? XD

@fabiosantoscode
Copy link
Contributor

That's a pretty sane fix. Out of curiosity @alexlamsl why not gate the whole drop_side_effect_free machinery behind that check?

@LongTengDao the fix should be pretty easy to implement! You already have the literal source code for it :) to be honest I don't really want to support the use case where uglified code is read by anything other than a JavaScript runtime.

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