Invalid minification of generators

[IJMacD]

test.js:

const a = {
    *gen () { yield 1; }
};
console.log([...a.gen()]);

Output:

[1]

Minify:

> npx minify@latest test.js

Minify Output:

var a={gen(){yield 1}};console.log(a.gen());

JS Error:

Uncaught SyntaxError: Unexpected number

Correct minified output:

var a={*gen(){yield 1}};console.log([...a.gen()]);

(Note missing asterisk, missing array literal, and spread operator.)

[coderaiser]

Please re-install Minify and add .minify.json with:

{
    "removeUselessSpread": false
}

Is it works for you?

[IJMacD]

Current output:

var a={*gen(){yield 1}};console.log(a.gen());

The generator function is now being correctly minified.

However, the output after execution is still incorrect.

Expected:

[1]

Actual:

Object [Generator] {}

Adding the .minify.json file makes no difference.

[coderaiser]

My mistake, here is valid config:

{
    "js": {
        "removeUselessSpread": false
    }
}

And better to use something like Array.from() to clearly show intention.

[IJMacD]

OK using this .minify.json produces correct output.

While Array.from() is a valid alternative the point here is not about coding style.

In its default configuration (with out .minify.json) Minify produces incorrect code. A minifier should never alter the runtime behaviour of code it transforms.

[coderaiser]

While Array.from() is a valid alternative the point here is not about coding style.

The thing not just in code style, with syntax [...fn()] you can get a lot of things so better to be concrete so:

• other developers can understand code faster;
• tools can understand code better;
• redundant patterns can be dropped out when they don't needed;

About default behavior let's look how many people will trap on this, and if they will be a lot, we can of course make this default behavior in a future.

[IJMacD]

I'm sorry I strongly disagree with you on this point.

The thing not just in code style

It is almost the definition of coding style. Two equivalent inputs produce identical results in un-minified code yet produce different results after minification.

[...a].map((x,i) => `${i}: ${x}`)

or

Array.from(a).map((x,i) => `${i}: ${x}`)

After minification:

a.map((x,i) => `${i}: ${x}`);

JS Error:

a.map is not a function

I think you misunderstand exactly what a could be. Array is only one possible option. a can be any iterable which is any object which implements the iterable protocol by defining Symbol.iterator or any of the built in iterable types String, TypedArray, Map, Set, and Segments or the arguments object or NodeList of DOM elements.

Whether you agree with the programmer's choice to use [...a] or not, a minifier should never alter the runtime behaviour of the code

[coderaiser]

I cannot reproduce this, after minification I have such results:

I'm talking about cognitive load, when you see such expression:

[...a].map((x,i) => `${i}: ${x}`)

you have variants:

• a is array and this spread has no sense;
• a is String and it is converted to array;
• a is arguments and it has sense;

Why developer should ask all this questions each time he see this expression? When you see Array.from() you understand that thing is convertion and most likely it is intentional.

Sometimes you merge arrays with:

const c = [
   ...a,
   ...b,
];

And then there is no need it ...b, what you left is:

const c = [
   ...a,
];

It is useless if you don't mutate Array. Same with objects.

a minifier should never alter the runtime behaviour of the code

The thing is minifier always alter runtime behavior, it changes const with vars, so there is no block scopes except functions, it changes if condition to logical expression and does other things.

But! I agree with you that should be ability to configure minifier and disable what not needed or goes against the current code style, and I'll be happy to simplify things and provide an easy way to make it possible.

[IJMacD]

The thing is minifier always alter runtime behavior, it changes const with vars, so there is no block scopes except functions, it changes if condition to logical expression and does other things.

Again, I think you misunderstand. The syntax will be different. That is expected and is the job of the minifier. However, the observable behaviour when executing the code should be identical.

Why developer should ask all this questions each time he see this expression?

He doesn't need to. What exactly a is, is irrelevant. All the developer needs to know is that a is iterable, and this expression will turn it into an array.

It's strange to me that you think the [...a,...b] syntax is fine but [...a] isn't. The developers on your team either know what [...X, ] does or they do not.

But all of this really isn't important because you're breaking the biggest rule of a minifier. A minifier shot not be opinionated. It should not take valid code and produce invalid code. It should not change the runtime behaviour of code that is passed to it.

[IJMacD]

I cannot reproduce this, after minification I have such results: Array.from(a).map((x,i) => `${i}: ${x}`)

Yes the second example produces that output, hence the two equivalent inputs produce two different outputs.

Ironically, you might consider adding a plugin to turn Array.from(a) into [...a] since they are equivalent and the length is 13 bytes vs 6 bytes.

[coderaiser]

Ironically, you might consider adding a plugin to turn Array.from(a) into [...a] since they are equivalent and the length is 13 bytes vs 6 bytes.

That's a good idea, why not https://github.com/coderaiser/putout/blob/master/packages/plugin-minify/README.md#convert-array-from-to-spread.

[coderaiser]

But all of this really isn't important because you're breaking the biggest rule of a minifier. A minifier shot not be opinionated. It should not take valid code and produce invalid code. It should not change the runtime behaviour of code that is passed to it.

There is no such a rule, but as I said, I'll add any options so you can produce stable code using Minify.

The options are added right now, and according to semantic versioning rule, changed options should be a breaking change, so at first I think we should get the full list of changed options and changed behavior before releasing a major version.