trullock / NUglify

NUglify is a HTML, JavaScript and CSS minification Library for .NET (fork of AjaxMin + new features)
Other
398 stars 80 forks source link

NUglify incorrectly removes empty setter function argument producing invalid JS code. #214

Closed rwasef1830 closed 3 years ago

rwasef1830 commented 3 years ago

Hello, NUglify is incorrectly removing the single mandatory parameter of setter functions when the setter function is empty, thus producing "Setter function must have a single formal argument" errors, and invalid JS that doesn't work in the browser:

Example:

export class MyClass {
    set property(value) {}
}

gets minified to:

export class MyClass{set property(){}}

Which is rejected by the browser.

trullock commented 3 years ago

Thanks for the report. This should be a straightforward fix

rwasef1830 commented 3 years ago

Hello, This bug is not fixed. In this code, the function argument gets elided incorrectly. You shouldn't check the name of the parameter being 'value', only that they are a single parameter. The parameter name can be anything and it is valid JS.

@trullock

Failing test input file: https://github.com/rwasef1830/NUglify/commit/040a0c13bd96b0941732982894523b6ff5673f7b

rwasef1830 commented 3 years ago

I notice also that the parameter name seems to be excluded from parameter renaming, I think that's not necessary as well. It can be involved in the normal parameter naming process as a normal parameter, just not removed.

trullock commented 3 years ago

I notice also that the parameter name seems to be excluded from parameter renaming, I think that's not necessary as well. It can be involved in the normal parameter naming process as a normal parameter, just not removed.

If this is true please open a new issue :)