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

Function names are not properly minified and causes a invalid syntax when ecma=5 is provided #1019

Closed
JayaKrishnaNamburu opened this issue Jul 1, 2021 · 3 comments
Labels

Comments

@JayaKrishnaNamburu
Copy link

JayaKrishnaNamburu commented Jul 1, 2021

Bug

Version ^5.7.0

{ keep_fnames: true, keep_classnames: true } 

These values are initially passed inside the terser-webpack-plugin. https://github.com/JayaKrishnaNamburu/terser-minifier-bug-reproduction/blob/main/webpack/webpack.config.prod.js#L18

I am using terser alongside webpack via terser-webpack-plugin and when built for production. The function is replaced with some for loop with a invalid syntax

terser input
Terser input was from a npm module. And the raw file can be found here https://unpkg.com/browse/@teleporthq/teleport-plugin-common@0.17.0/dist/esm/index.js

// packages/teleport-plugin-common/src/utils/hast-utils.ts
var hast_utils_exports = {};
teleport_plugin_common_dist_esm_export(hast_utils_exports, {
  addAttributeToNode: () => addAttributeToNode,
  addBooleanAttributeToNode: () => addBooleanAttributeToNode,
  addChildNode: () => addChildNode,
  addClassToNode: () => addClassToNode,
  addTextNode: () => addTextNode
});
var addBooleanAttributeToNode = (node, key, value = true) => {
  node.properties[key] = value === true ? "" : false;
};
var addAttributeToNode = (node, key, value) => {
  node.properties[key] = value;
};
var addClassToNode = (node, className) => {
  node.properties.class = className;
};
var addChildNode = (node, child) => {
  node.children.push(child);
};
var addTextNode = (node, text) => {
  node.children.push(createTextNode(text));
};

terser output or error

       teleport_plugin_common_dist_esm_export(_N, {
            addAttributeToNode: ()=>addAttributeToNode,
            addBooleanAttributeToNode: ()=>addBooleanAttributeToNode,
            addChildNode: ()=>addChildNode,
            addClassToNode: ()=>addClassToNode,
            addTextNode: ()=>addTextNode
        });
        for (var yN, vN, addBooleanAttributeToNode = (pT,xT,OT=!0)=>{
            pT.properties[xT] = !0 === OT && ""
        }
        , addAttributeToNode = (pT,xT,OT)=>{
            pT.properties[xT] = OT
        }
        , addClassToNode = (pT,xT)=>{
            pT.properties.class = xT
        }
        , addChildNode = (pT,xT)=>{
            pT.children.push(xT)
        }
        , addTextNode = (pT,xT)=>{
            pT.children.push(createTextNode(xT))
        }

I Recorded a small video on the behaviour and it can be found from the initial issue which i raised here webpack-contrib/terser-webpack-plugin#410 in the webpack plugin itself.

Screen.Recording.2021-07-01.at.11.30.01.PM.mov

Git Repository to reproduce the bug
https://github.com/JayaKrishnaNamburu/terser-minifier-bug-reproduction

Steps to reproduce from the Git repository
https://github.com/JayaKrishnaNamburu/terser-minifier-bug-reproduction#readme

@JayaKrishnaNamburu
Copy link
Author

JayaKrishnaNamburu commented Jul 2, 2021

Cli version 5.7.0

I was able to reproduce this outside webpack, by using terser-cli. Here is the un-optimised chunk
https://drive.google.com/file/d/1ftvrauvf2nUITcWLchzv_DJoFQqPRTIz/view?usp=sharing

Here is the flags to be used in cli to get a chunk that generates the invalid syntax.

terser 380.7ed8f3df.js<chunk-name> -o compress.js -c ecma=5 --keep_classnames --keep_fnames

PS: The ecma=5 is important since that what causes the issue, if we remove ecma=5 then everything works just fine. But i think, the provided chunk might have some bundled es6 stuff too. Is that's what causing the breaking syntax ?

Then load the chunk in browser

@JayaKrishnaNamburu JayaKrishnaNamburu changed the title Function names are not properly minified and causes a invalid syntax Function names are not properly minified and causes a invalid syntax when ecma=5 is provided Jul 5, 2021
@fabiosantoscode
Copy link
Collaborator

Ok, I got to the bottom of this.

With terser input.js -c and input:

var var1 = somethingUnknown1;
var var2 = (key) => key in somethingUnknown2

for (var i = 0; i < chars.length; i++) {
    loopyLoop()
}

Terser will join var1, var2 and i into just one decl to save space.

for (var var1 = somethingUnknown1, var2 = key => key in somethingUnknown2, i = 0; i < chars.length; i++) loopyLoop();

Because of key in somethingUnknown2 is there, it mixes up the parser, which assumed the function was just key => key and then it sees in, assuming it's a for-in loop.

@fabiosantoscode
Copy link
Collaborator

In the end it was just a matter of adding parens. This was already being done for other expressions, but not arrow functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants