Taritsyn / WebMarkupMin

The Web Markup Minifier (abbreviated WebMarkupMin) - a .NET library that contains a set of markup minifiers. The objective of this project is to improve the performance of web applications by reducing the size of HTML, XHTML and XML code.
Apache License 2.0
449 stars 48 forks source link

NUglifyJsMinifierFactory with RenamePairs does not consitently rename properties and methods of JS classes #175

Open MarcelVersteeg opened 1 day ago

MarcelVersteeg commented 1 day ago

Consider JavaScript like this:

class MyClass
{
    constructor(field)
    {
        this.field = field;
    }

    method()
    {
        field++;
    }

    get property()
    {
        return field;
    }
}

var myVar = new MyClass();
myVar.method();
var property = myVar.property;

Then I configure the WebMarkupMin minification as follows (irrelevant options left out for brevity):

WebMarkupMinServicesBuilder minificationBuilder = services.AddWebMarkupMin(options =>
        {
            ...
        });

// Add the HTML minification
minificationBuilder.AddHtmlMinification(options =>
        {
            options.JsMinifierFactory = new NUglifyJsMinifierFactory(new NUglifyJsMinificationSettings
                    {
                        LocalRenaming = LocalRenaming.CrunchAll,
                        PreserveFunctionNames = false,
                        PreserveImportantComments = false,
                        QuoteObjectLiteralProperties = false,
                        RemoveFunctionExpressionNames = true,
                        RemoveUnneededCode = true,
                        RenamePairs = @"MyClass=a,field=b,method=c,property=d,myVar=e",
                        ReorderScopeDeclarations = true,
                        StrictMode = false,
                        StripDebugStatements = true,
                        TermSemicolons = false,
                        WarningLevel = int.MaxValue
                    });
        });

Now, the resulting uglified JavaScript will become (indentation kept for readability):

class a
{
    constructor(b)
    {
        this.b = b;
    }

    method()
    {
        b++;
    }

    get property()
    {
        return b;
    }
}

var e = new a();
e.c();
var d = e.d

This code does not run correctly, as the actual method and property in the class are not renamed, but the locations where they are called are renamed.

Preferrably, the methods and properties of the class are also renamed (just like the fields), but at least the renaming should be consistent to not break the uglified JavaScript

Taritsyn commented 22 hours ago

Hello, Marcel!

I tried to minify your code by using the original library (NUglify):

using NUglify;
using NUglify.JavaScript;

namespace TestNUglify
{
    class Program
    {
        static void Main(string[] args)
        {
            const string input = @"class MyClass
{
    constructor(field)
    {
        this.field = field;
    }

    method()
    {
        this.field++;
    }

    get property()
    {
        return this.field;
    }
}

var myVar = new MyClass();
myVar.method();
var property = myVar.property;";
            var settings = new CodeSettings
            {
                OutputMode = OutputMode.MultipleLines,

                // Your settings
                LocalRenaming = LocalRenaming.CrunchAll,
                PreserveFunctionNames = false,
                PreserveImportantComments = false,
                QuoteObjectLiteralProperties = false,
                RemoveFunctionExpressionNames = true,
                RemoveUnneededCode = true,
                RenamePairs = @"MyClass=a,field=b,method=c,property=d,myVar=e",
                ReorderScopeDeclarations = true,
                StrictMode = false,
                StripDebugStatements = true,
                TermSemicolons = false,
                WarningLevel = int.MaxValue
            };

            UglifyResult minifiedResult = Uglify.Js(input, settings);
            if (!minifiedResult.HasErrors)
            {
                Console.ForegroundColor = ConsoleColor.Green;
                Console.WriteLine("Minification success!");
                Console.WriteLine();

                Console.ForegroundColor = ConsoleColor.White;
                Console.WriteLine(minifiedResult.Code);
            }
            else
            {
                Console.ForegroundColor = ConsoleColor.Red;
                Console.WriteLine("Minification failed!");
                Console.WriteLine();

                Console.ForegroundColor = ConsoleColor.White;

                foreach (UglifyError error in minifiedResult.Errors)
                {
                    Console.WriteLine(error);
                }
            }
        }
    }
}

And got a similar result:

var e,
    d;
class a
{
    constructor(b)
    {
        this.b = b
    }
    method()
    {
        this.b++
    }
    get property()
    {
        return this.b
    }
}
e = new a;
e.c();
d = e.d

If you refuse the RenamePairs = @"MyClass=a,field=b,method=c,property=d,myVar=e" setting, then you will get a working code:

var myVar,
    property;
class MyClass
{
    constructor(n)
    {
        this.field = n
    }
    method()
    {
        this.field++
    }
    get property()
    {
        return this.field
    }
}
myVar = new MyClass;
myVar.method();
property = myVar.property

If you are not satisfied with this result, then I recommend that you report about this problem to author of the original library.

Taritsyn commented 21 hours ago

By the way, you can also get a working code by using the ManualRenamesProperties = false setting.

Taritsyn commented 8 hours ago

The effect that you are trying to get by using the RenamePairs property can be achieved without manual settings. You just need to wrap your code in a IIFE:

(function () {
    class MyClass
    {
        constructor(field)
        {
            this.field = field;
        }

        method()
        {
            this.field++;
        }

        get property()
        {
            return this.field;
        }
    }

    var myVar = new MyClass();
    myVar.method();
    var property = myVar.property;
})();

And after minification you will get the following result:

(function()
{
    var n,
        i;
    class t
    {
        constructor(n)
        {
            this.field = n
        }
        method()
        {
            this.field++
        }
        get property()
        {
            return this.field
        }
    }
    n = new t;
    n.method();
    i = n.property
})()
MarcelVersteeg commented 8 hours ago

Hi @Taritsyn,

Thank you for your reply. Both solutions that you gave prevent uglyfying the properties and methods of a class, which is what I want.

I have submitted an issue in the original repo (https://github.com/trullock/NUglify/issues/406)

MarcelVersteeg commented 8 hours ago

The effect that you are trying to get by using the RenamePairs property can be achieved without manual settings. You just need to wrap your code in a IIFE:

(function () {
    class MyClass
    {
        constructor(field)
        {
            this.field = field;
        }

        method()
        {
            this.field++;
        }

        get property()
        {
            return this.field;
        }
    }

    var myVar = new MyClass();
    myVar.method();
    var property = myVar.property;
})();

And after minification you will get the following result:

(function()
{
    var n,
        i;
    class t
    {
        constructor(n)
        {
            this.field = n
        }
        method()
        {
            this.field++
        }
        get property()
        {
            return this.field
        }
    }
    n = new t;
    n.method();
    i = n.property
})()

Thank you for this reply. I will have a look at this.

MarcelVersteeg commented 8 hours ago

Hi @Taritsyn,

I took a closer look at the result of the minification you posted after putting the code inside an IIFE, but still the method and property are not minified, which is what I want to achieve anyway.

So let's see what the developer's of NUglify say.