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

V2 Scope hoisting seems to break renaming #4687

Closed
jfrconley opened this issue Jun 3, 2020 · 13 comments · Fixed by #4690
Closed

V2 Scope hoisting seems to break renaming #4687

jfrconley opened this issue Jun 3, 2020 · 13 comments · Fixed by #4690

Comments

@jfrconley
Copy link
Contributor

jfrconley commented Jun 3, 2020

🐛 bug report

Enabling scope hoisting seems to break variable renaming resulting in a significantly increased bundle size.

This may be related to #3664, but I have verified that I have @babel/travers at 7.10.1

🎛 Configuration (.babelrc, package.json, cli command)

parcel build --target lambda --no-source-maps dist/index.js --detailed-report
{
  "lambda": "bundle/index.js",
  "targets": {
    "lambda": {
      "context": "node",
      "sourceMap": false,
      "includeNodeModules": {
        "@acuris/aws-es-connection": false,
        "@elastic/elasticsearch": false,
        "sharp": false,
        "aws-sdk": false,
        "jsonata": false,
        "pino-pretty": false,
        "jmespath": false
      }
    }
  }
}

🤔 Expected Behavior

Enabling scope hoisting should have shortest possible top level variable names, reducing overall bundle size.

😯 Current Behavior

While scope hoisting itself seems to be working, top level values all have super long generated names (ex: $b10acc16e6462fee040168c6ed2f0$var$actualClose), significantly increasing bundle size. In my case, the scope hoisted size was double that of simple bundling.

🌍 Your Environment

Software Version(s)
Parcel 2.0.0-nightly.277
Node 12
npm/Yarn 1.22.4
Operating System Ubuntu 20.04
@jfrconley jfrconley changed the title Scope hoisting seems to break renaming V2 Scope hoisting seems to break renaming Jun 3, 2020
@mischnic
Copy link
Member

mischnic commented Jun 3, 2020

hile scope hoisting itself seems to be working, top level values all have super long generated names (ex: $b10acc16e6462fee040168c6ed2f0$var$actualClose), significantly increasing bundle size. In my case, the scope hoisted size was double that of simple bundling.

I'm fairly confident that you have an eval somewhere in the bundle, which deopts Terser.

@jfrconley
Copy link
Contributor Author

Just confirmed that there are no calls to eval or use of the Function constructor anywhere in my bundle.

@mischnic
Copy link
Member

mischnic commented Jun 3, 2020

Strange, even if you build with --no-minify?

@jfrconley
Copy link
Contributor Author

Yes, just double checked. The minify option seems to only apply to contents of modules, not the bundle

@mischnic
Copy link
Member

mischnic commented Jun 3, 2020

The minify option seems to only apply to contents of modules, not the bundle

Parcel 1 ran Terser per file/asset, but Parcel 2 does run it on the concatenated bundle

@jfrconley
Copy link
Contributor Author

jfrconley commented Jun 3, 2020

Is there a documented list of things that cause terser to bail out? I'm testing this on a very small bundle (13 KB) and can't find anything that seems likely.

EDIT: Tried running terser directly on output, and it did nothing. Seems like it's bailing out for some reason.

@jfrconley
Copy link
Contributor Author

I figured it out. Terser does not mangle top level names by default, but requires the toplevel option to be set. Parcel should probably do this by default when scope hoisting.

https://terser.org/docs/cli-usage#cli-mangle-options

@mischnic
Copy link
Member

mischnic commented Jun 3, 2020

We should:

toplevel:
bundle.env.outputFormat === 'esmodule' ||
bundle.env.outputFormat === 'commonjs',

Your outputformat should be commonjs, right?

@jfrconley
Copy link
Contributor Author

That only applies to compression, not mangling. The issue goes away when I have .terserrc with the following config.

{
	"mangle": {
		"toplevel": true
	}
}

@mischnic
Copy link
Member

mischnic commented Jun 3, 2020

👍 Thanks for investigating!

@fregante
Copy link
Contributor

fregante commented Apr 8, 2021

It seems that the suggested way to handle this issue is to enable variable name mangling through minification, which makes variables less readable rather than more readable.

Can Parcel do what Rollup does to generate readable code? My code has been rejected by a store due to “obfuscation”

DCCF3E01-0D3C-4839-B9BE-043940D5C1FC

@mischnic
Copy link
Member

mischnic commented Apr 8, 2021

@fregante That's very unfortunate, can you open a new issue for that? Maybe we can try to shorten the names

@fregante
Copy link
Contributor

fregante commented Apr 8, 2021

Thanks! Done

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

Successfully merging a pull request may close this issue.

3 participants