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

Optimize simple classes in private scope #3679

Closed
rentalhost opened this issue Jan 8, 2020 · 6 comments · Fixed by #3680
Closed

Optimize simple classes in private scope #3679

rentalhost opened this issue Jan 8, 2020 · 6 comments · Fixed by #3680

Comments

@rentalhost
Copy link

Bug report or feature request? Feature request

Uglify version (uglifyjs -V) 3.7.4

JavaScript input

(function () {
    "use strict";

    var Test = function() {};

    Test.main = function() {
        console.log("Hello world!");
    };

    Test.main();
})();

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

uglifyjs input.js

JavaScript output or error produced.

(function(){"use strict";var Test=function(){};Test.main=function(){console.log("Hello world!")};Test.main()})();

Expected:

console.log("Hello world!");

Note that Test class have only a single method, that is called statically a single time. It is too private, so it can't accessed by external scope.

A simplest example:

(function () {
    function Test() {
        console.log("Hello world!");
    };

    Test();
})();
@alexlamsl
Copy link
Collaborator

@rentalhost
Copy link
Author

Actually I do not see reason (except for the possible complexity or by the scope where it was declared) to the output not be optimized to something like:

console.log('function'); // or
console.log(typeof Function);

Step-by-step:

        function f() {
            function f() {} // f() is declared
            function g() {
                return this;
            }
            f.g = g; // Unique f() property
            return f.g(); // Unique usage of f.g property
            // So f() could be dropped to use g directly.
        }
        console.log(typeof f());
        function f() {
            function g() {
                return this;
            }
            return g(); // g() will return this (from a very simple Function, that in this 
                        // scope have the same construction of the external f()). So we can
                        // replace it direcly with this (due the next unique usage).
        }
        console.log(typeof f());
        function f() {
            return this;
        }
        console.log(typeof f()); // f() will return this (that is a Function), 
                                 // so it could be modified.
        console.log(typeof Function); // typeof Function === 'function'
        console.log('function');

I am considering here that optimization can take into account how functions and variables are exposed to the global scope, and that it is possible to identify how many times a given function is used and its possible return.

So for instance (considering that the above steps are possible):

        function f() {
            function f() {} 
            function g() {
                return this;
            }
            f.g = g; 
            return f.g(); 
        }

        console.log(typeof f());
        console.log(typeof f());
        console.log(typeof f());

Could be safety optimized to:

        console.log('function');
        console.log('function');
        console.log('function');

Although f() has been called multiple times, it is possible to predict its identical return each time it is called.

@alexlamsl
Copy link
Collaborator

This doesn't do what you think it does:

function f() {
    return this;
}
console.log(typeof f());
$ echo function f(){return this} console.log(typeof f()) | node
object

So it certainly isn't a valid optimisation step.

@rentalhost
Copy link
Author

Really, but anyway, it could be optimized to "object" (or maybe not be optimized if return this is used).

alexlamsl added a commit to alexlamsl/UglifyJS that referenced this issue Jan 9, 2020
@alexlamsl
Copy link
Collaborator

Well, it couldn't since that will change the result of the output which violates the basic assumption of operation.

I've put together a Pull Request that will work on your original example − feel free to improve upon it.

@rentalhost
Copy link
Author

Perfect!

alexlamsl added a commit that referenced this issue Jan 9, 2020
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