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

unknown directives retained #3166

Closed
kzc opened this issue May 31, 2018 · 3 comments · Fixed by #3203
Closed

unknown directives retained #3166

kzc opened this issue May 31, 2018 · 3 comments · Fixed by #3203

Comments

@kzc
Copy link
Contributor

kzc commented May 31, 2018

Bug report or feature request?

feature request

Uglify version (uglifyjs -V)

uglify-js 3.3.28

JavaScript input

$ bin/uglifyjs -V
uglify-js 3.3.28

$ echo '"use strict";"A";function f(){"use strict";"B"}f();f();' | bin/uglifyjs -c toplevel
"use strict";"A";function f(){"B"}f(),f();

An older uglify version:

$ uglifyjs -V
uglify-js 2.7.5

$ echo '"use strict";"A";function f(){"use strict";"B"}f();f();' | uglifyjs -c
WARN: Dropping side-effect-free statement [-:1,43]
"use strict";"A";function f(){}f(),f();

Is it worth adding a new "directives" compress options defaulting to true to drop directives other than "use strict" and "use asm"? There might be obscure use cases with custom directives out in the wild. Perhaps this option could default to allowable directives - i.e.: -c directives=["use strict", "use asm"].

@kzc
Copy link
Contributor Author

kzc commented May 31, 2018

It appears that older versions of uglify also retained the unknown directives within functions - the string "B" above did not have a trailing semicolon, so it was not considered a directive.

$ uglifyjs -V
uglify-js 2.7.5

$ echo '"use strict";"A";function f(){"use strict";"B";}f();f();' | uglifyjs -c passes=9
"use strict";"A";function f(){"B"}f(),f();

Whether unknown directives were retained historically is not important. The question is how should it work in the Compressor.

@alexlamsl
Copy link
Collaborator

A new directives flag which retains only "use asm" and "use strict" (case sensitive) sounds fine to me - PR is welcome.

@kzc
Copy link
Contributor Author

kzc commented May 31, 2018

Was first noticed while debugging some ESTree AST. Must not be a real issue in the wild. Both babel-minify and Google Closure retain the unknown directives - albeit with a warning for Closure. Maybe they are trying to be future compatible.

If I add such a compress option to terser I'll post a link here.

alexlamsl added a commit to alexlamsl/UglifyJS that referenced this issue Jun 28, 2018
alexlamsl added a commit that referenced this issue Jun 28, 2018
aleclarson pushed a commit to aleclarson/UglifyJS2 that referenced this issue Jul 1, 2018
fabiosantoscode referenced this issue in terser/terser Jul 5, 2018
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

Successfully merging a pull request may close this issue.

2 participants