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

fix ie8 option alias #5103

Merged
merged 1 commit into from
Jul 26, 2021
Merged

fix ie8 option alias #5103

merged 1 commit into from
Jul 26, 2021

Conversation

alexlamsl
Copy link
Collaborator

fixes #5102

@kzc
Copy link
Contributor

kzc commented Jul 26, 2021

The PR is not quite right. Legacy ie8 sub-options of output, compress and mangle are not handled correctly. For example, these two commands should produce:

$ echo 'function aaa(t) { return t.default; }' | bin/uglifyjs -O ie8
function aaa(t){return t["default"]}
$ echo 'function aaa(t) { return t.default; }' | bin/uglifyjs -O ie
function aaa(t){return t["default"]}

I suggest something like the following, which also passes npm test:

diff --git a/lib/compress.js b/lib/compress.js
index dc171ae..8f5cbe9 100644
--- a/lib/compress.js
+++ b/lib/compress.js
@@ -72,3 +72,4 @@ function Compressor(options, false_by_default) {
         hoist_vars      : false,
-        ie              : false,
+        ie              : options && options.ie8 || false,
+        ie8             : options && options.ie || false,
         if_return       : !false_by_default,
diff --git a/lib/minify.js b/lib/minify.js
index d3394b4..4a4243a 100644
--- a/lib/minify.js
+++ b/lib/minify.js
@@ -78,4 +78,4 @@ function minify(files, options) {
             enclose: false,
-            ie: false,
-            ie8: false,
+            ie: options && options.ie8 || false,
+            ie8: options && options.ie || false,
             keep_fnames: false,
@@ -99,3 +99,4 @@ function minify(files, options) {
         if (options.annotations !== undefined) set_shorthand("annotations", options, [ "compress", "output" ]);
-        if (options.ie || options.ie8) set_shorthand("ie", options, [ "compress", "mangle", "output" ]);
+        if (options.ie) set_shorthand("ie", options, [ "compress", "mangle", "output" ]);
+        if (options.ie8) set_shorthand("ie8", options, [ "compress", "mangle", "output" ]);
         if (options.keep_fnames) set_shorthand("keep_fnames", options, [ "compress", "mangle" ]);
@@ -110,2 +111,3 @@ function minify(files, options) {
                 ie: false,
+                ie8: false,
                 keep_fnames: false,
diff --git a/lib/output.js b/lib/output.js
index 0cf379d..d41f053 100644
--- a/lib/output.js
+++ b/lib/output.js
@@ -58,3 +58,4 @@ function OutputStream(options) {
         galio            : false,
-        ie               : false,
+        ie               : options && options.ie8 || false,
+        ie8              : options && options.ie || false,
         indent_level     : 4,
diff --git a/lib/scope.js b/lib/scope.js
index 2219b89..35684b7 100644
--- a/lib/scope.js
+++ b/lib/scope.js
@@ -120,3 +120,4 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options) {
         cache: null,
-        ie: false,
+        ie: options && options.ie8 || false,
+        ie8: options && options.ie || false,
     });
@@ -574,3 +575,4 @@ function _default_mangler_options(options) {
         eval        : false,
-        ie          : false,
+        ie          : options && options.ie8 && false,
+        ie8         : options && options.ie && false,
         keep_fnames : false,

@alexlamsl
Copy link
Collaborator Author

That's intentional − I'm only keeping the alias for the top-level option.

@fengmk2
Copy link

fengmk2 commented Jul 26, 2021

Temporary fixes by cnpm/bug-versions#147

@kzc
Copy link
Contributor

kzc commented Jul 26, 2021

That's intentional − I'm only keeping the alias for the top-level option.

Why break backwards compatibility when it's so simple to resolve?

@alexlamsl
Copy link
Collaborator Author

Why break backwards compatibility when it's so simple to resolve?

Because only the top-level option is documented for general use.

If you are using the individual sub-options, you are either advanced enough to figure out what happened and delete one character, or you are using it by mistake since you need all of them for proper browser compatibility.

Either case, I am not spraying legacy code all over.

@alexlamsl alexlamsl merged commit c80eabd into mishoo:master Jul 26, 2021
@alexlamsl alexlamsl deleted the issue-5102 branch July 26, 2021 08:45
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 this pull request may close these issues.

ie8 option is not working after version 3.14.0
3 participants