Update Readme.md `mangle` settings #2286

snuggs · 2017-08-22T19:37:13Z

These are based off https://github.com/Perlmint/UglifyJS2/blob/0976fc3e4b303be6de8a366f8406d8906b5521e1/lib/minify.js#L54-L61
And had no documentation update from this issue #1753 and this PR #1851.

Discovered when working on this Pull Request devpunks/snuggsi#78

Please let me know where i can better update copy text. Difficult to know what the flags do without a readme for the flags. ;-)

Thanks!

/cc @brandondees @robcole @angelocordon

@brandondees

These are based off https://github.com/Perlmint/UglifyJS2/blob/0976fc3e4b303be6de8a366f8406d8906b5521e1/lib/minify.js#L54-L61 And had no documentation update from this issue #1753 and this PR #1851. Discovered when working on this Pull Request devpunks/snuggsi#78 Please let me know where i can better update copy text. Difficult to know what the flags do without a readme for the flags. ;-) Thanks! /cc @brandondees @robcole @angelocordon

kzc · 2017-08-22T19:45:48Z

README.md

@@ -64,6 +64,13 @@ a double dash to prevent input files being used as option arguments:
                                              not used.
    -m, --mangle [options]      Mangle names/specify mangler options:
                                `reserved`  List of names that should not be mangled.
+                                `cache` Boolean for cache determination.


This is not a boolean.

Remove cache option. See --name-cache flag instead.

kzc · 2017-08-22T19:46:34Z

README.md

@@ -64,6 +64,13 @@ a double dash to prevent input files being used as option arguments:
                                              not used.
    -m, --mangle [options]      Mangle names/specify mangler options:
                                `reserved`  List of names that should not be mangled.
+                                `cache` Boolean for cache determination.
+                                `eval` Boolean for mangling `eval` statements.


This description is incorrect. See existing description.

kzc · 2017-08-22T19:47:10Z

README.md

+                                `ie8` Mangle support for ie8.
+                                `keep_fnames` Keep function names.
+                                `properties` Mangle properties.
+                                `safari10` Boolean flag used to bypass Safari 10 `let` bug.


This option only exists on the harmony branch for uglify-es.

kzc · 2017-08-22T19:47:36Z

README.md

+                                `keep_fnames` Keep function names.
+                                `properties` Mangle properties.
+                                `safari10` Boolean flag used to bypass Safari 10 `let` bug.
+                                `toplevel` Boolean flag to determine toplevel.                                


Incorrect description. Please use existing.

kzc · 2017-08-22T19:48:52Z

README.md

+
+- `ie8` (default `false`). Mangle support for ie8.
+
+- `properties` (default `false`). Mangle properties.


This is a sub-option object with options of its own. See mangle properties.

kzc · 2017-08-22T19:49:02Z

README.md

@@ -745,9 +752,16 @@ If you're using the `X-SourceMap` header instead, you can just omit `sourceMap.u
  Useful for code relying on `Function.prototype.name`. See also: the `keep_fnames`
  [compress option](#compress-options).

- `eval` (default `false`). Pass `true` to mangle names visible in scopes
-  where `eval` or `with` are used.
+- `cache` (default `false`). Boolean for cache determination.


This is not a boolean.

@kzc please provide the copy you would like to see and I will update. Again it was like a cat and mouse game chasing docs.

Remove the cache option altogether. It is accessed via the flag:

--name-cache <file> File to hold mangled name mappings.

My mistake - this is the API section. cache is a cache object.

snuggs · 2017-08-22T19:52:42Z

Also out of curiosity should this be on the harmony branch?

https://github.com/mishoo/UglifyJS2/tree/harmony

kzc · 2017-08-22T19:54:03Z

README.md

+                                `eval` Boolean for mangling `eval` statements.
+                                `ie8` Mangle support for ie8.
+                                `keep_fnames` Keep function names.
+                                `properties` Mangle properties.


properties is not accessible from the CLI. Use --mangle-props instead.

kzc · 2017-08-22T20:01:39Z

Also out of curiosity should this be on the harmony branch?

The options pertaining to uglify-js@3 should be on master. The extra options pertaining to uglify-es should be on harmony. uglify-js@3 only handles ES5. uglify-es handles ES6+ and is a superset of uglify-js@3.

kzc · 2017-08-22T20:04:25Z

README.md

+
+- `properties` (default `false`). Mangle properties.
+
+- `safari10` (default `false`). Boolean flag used to bypass Safari 10 `let` bug.


This is already documented in the harmony branch for uglify-es.

kzc · 2017-08-22T20:06:30Z

uglify-es mangle options documented here:

https://github.com/mishoo/UglifyJS2/tree/harmony#mangle-options

kzc · 2017-08-22T20:10:51Z

I think the CLI mangle options should just link to the above link rather than repeat all the mangle sub-options.

snuggs · 2017-08-22T22:29:42Z

@kzc to be clear in what you are saying you are telling me to link to the mangle options in the harmony branch? That wouldn't make sense to me. I get the duplication but the harmony branch is itself somewhat of a big "duplication". Why are we linking to the harmony branch when only 1 of the flags is harmony related (i believe)? Also this is just a footgun for whenever the branch does (finally) get merged. Please advise.

kzc · 2017-08-22T22:51:25Z

ES5 uglify-js releases are done from master. ES6+ uglify-es releases are done from harmony. The documentation for each should reside in the appropriate branch.

There's no need to redundantly enumerate all the sub-options in the CLI sections. Just make a PR on master linking the CLI mangle section to the API mangle options. Could also do the same for parse, compress, and output options. Add any missing sub-options to README on master in the API section. The changes will be merged to harmony before next release.

uglify-es might be merged into uglify-js one day - or perhaps it won't - it's up to @alexlamsl.

snuggs · 2017-08-22T23:57:21Z

Oh i see @kzc I shall address the others. I'm fairly new at uglifyjs but pretty good at creating technical documentation

I know you mentioned it before but can you provide some laundry list of what i should look at? I thought was a quick fix for safari10 but looks like the README could use a little house keeping (what README doesn't). I don't mind contributing where I can but will need to address tomorrow as my day is done. Just to give you a heads up. Nothing more awkward than lingering PRs. ;-)

snuggs · 2017-08-23T00:22:18Z

Also @kara @alexlamsl wanted to say I owe ya some coffee at least for making such an awesome project. We went through EVERYTHING in the past 6 months. UglifyES is helping us deliver a 1kB package to the browser Even fits within an ethernet frame. :-) I thought Uglify would do transpilation for us but we decided to go with buble. Anticipating build pipelines to decrease by about 80% over the coming years. But see UglifyJS definitely within the remaining 20% (as far as we're concerned).

Again thanks so much for the contributions! 🙏 /cc @brandondees

kzc · 2017-08-23T01:11:16Z

@snuggs Thanks for the kind words.

can you provide some laundry list of what i should look at? I thought was a quick fix for safari10 but looks like the README could use a little house keeping

safari10 is already in the mangle options of harmony a.k.a uglify-es. Other documented harmony/uglify-es-specific options include (in no particular order or section): ecma, keep_classnames, arrows, unsafe_arrows. (Others?)

If you think you can make the documentation more coherent, go for it. Off the top of my head:

We were hoping someone would alphabetize the compress options on master which would subsequently be merged into harmony.
People always like more examples of CLI and API use. Perhaps an example that you might have had problems with.
There's also a bash quoting issue in CLI doesn't add sourceMappingURL line to minified output #2284 (comment) that could be better documented. If you have time you could try running all the CLI examples in bash to see if they work as documented. Handled in Fix CLI source-maps examples #2291
Please don't change the headings as many projects link to them.

alexlamsl · 2017-08-23T05:03:59Z

@snuggs glad you find uglify-js useful, and thanks for tackling documentation!

I shall let you and @kzc polish this and wait for the ✅

alexlamsl · 2017-10-22T12:25:37Z

Closing as this is superseded by recent updates to README.md

snuggs · 2017-10-22T13:52:56Z

Care to drop a sha @alexlamsl? Thanks for the movement on this... I recently have been dealing with health issues from both my mothers. I'll get in where I fit in on the next go round! 🙏 💛

alexlamsl · 2017-10-22T14:09:19Z

@snuggs the relevant PRs are #2355, #2356 & #2374

Best wishes & see you around soon!

kzc reviewed Aug 22, 2017

View reviewed changes

alexlamsl force-pushed the master branch from 3f5a7b7 to 7e3e9da Compare September 20, 2017 20:43

alexlamsl closed this Oct 22, 2017

snuggs deleted the patch-1 branch October 22, 2017 13:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Readme.md `mangle` settings #2286

Update Readme.md `mangle` settings #2286

snuggs commented Aug 22, 2017

kzc Aug 22, 2017

kzc Aug 22, 2017

kzc Aug 22, 2017

kzc Aug 22, 2017

kzc Aug 22, 2017

kzc Aug 22, 2017

kzc Aug 22, 2017

snuggs Aug 22, 2017

kzc Aug 22, 2017

kzc Aug 22, 2017

snuggs commented Aug 22, 2017

kzc Aug 22, 2017

kzc commented Aug 22, 2017

kzc Aug 22, 2017

kzc commented Aug 22, 2017

kzc commented Aug 22, 2017

snuggs commented Aug 22, 2017

kzc commented Aug 22, 2017 •

edited

Loading

snuggs commented Aug 22, 2017 •

edited

Loading

snuggs commented Aug 23, 2017 •

edited

Loading

kzc commented Aug 23, 2017 •

edited

Loading

alexlamsl commented Aug 23, 2017

alexlamsl commented Oct 22, 2017

snuggs commented Oct 22, 2017 •

edited

Loading

alexlamsl commented Oct 22, 2017


		- `ie8` (default `false`). Mangle support for ie8.

		- `properties` (default `false`). Mangle properties.


		- `properties` (default `false`). Mangle properties.

		- `safari10` (default `false`). Boolean flag used to bypass Safari 10 `let` bug.

Update Readme.md mangle settings #2286

Update Readme.md mangle settings #2286

Conversation

snuggs commented Aug 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snuggs commented Aug 22, 2017

Choose a reason for hiding this comment

kzc commented Aug 22, 2017

Choose a reason for hiding this comment

kzc commented Aug 22, 2017

kzc commented Aug 22, 2017

snuggs commented Aug 22, 2017

kzc commented Aug 22, 2017 • edited Loading

snuggs commented Aug 22, 2017 • edited Loading

snuggs commented Aug 23, 2017 • edited Loading

kzc commented Aug 23, 2017 • edited Loading

alexlamsl commented Aug 23, 2017

alexlamsl commented Oct 22, 2017

snuggs commented Oct 22, 2017 • edited Loading

alexlamsl commented Oct 22, 2017

Update Readme.md `mangle` settings #2286

Update Readme.md `mangle` settings #2286

kzc commented Aug 22, 2017 •

edited

Loading

snuggs commented Aug 22, 2017 •

edited

Loading

snuggs commented Aug 23, 2017 •

edited

Loading

kzc commented Aug 23, 2017 •

edited

Loading

snuggs commented Oct 22, 2017 •

edited

Loading