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

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

Open
MarcelVersteeg opened this issue Oct 1, 2024 · 6 comments

Comments

@MarcelVersteeg
Copy link

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
Copy link
Owner

Taritsyn commented Oct 1, 2024

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
Copy link
Owner

Taritsyn commented Oct 1, 2024

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

@Taritsyn
Copy link
Owner

Taritsyn commented Oct 2, 2024

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
Copy link
Author

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 (trullock/NUglify#406)

@MarcelVersteeg
Copy link
Author

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
Copy link
Author

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.

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

2 participants