OpenUserJS.org icon indicating copy to clipboard operation
OpenUserJS.org copied to clipboard

Test and support new option for *express-minify* with keeping comments

Open Martii opened this issue 11 years ago • 33 comments

See also:

  • [x] SummerWish/express-minify#15
  • [x] https://github.com/OpenUserJS/OpenUserJS.org/commit/1b595bb6a986a781fa87f04cc27e31a011b81dbb
  • [x] mishoo/UglifyJS2#583 (fixed with mishoo/UglifyJS2#608 ... retested with known derivations to the UserScript metadata block including tab characters... ~~not released yet though~~ v2.6.1+)
  • [ ] mishoo/UglifyJS2#448 (git://github.com/mishoo/UglifyJS2#harmony and git://github.com/OpenUserJs/UglifyJS2#harmony-named+cdba43c) (WIP)
  • [ ] mishoo/UglifyJS2#88 (not blocking yet) ... may be fixed
  • [x] SummerWish/express-minify#42
  • [x] SummerWish/express-minify#44
  • [x] Establish the core UI elements with bootstrap
  • [x] Establish testing methodology (loading two instances of UglifyJS2 and separated out handling of package .js vs userscript/lib scripts)
  • [x] Establish permanent url usage
  • [ ] Custom config UglifyJS2 options. (partially started)
  • [x] mishoo/UglifyJS2#935 with PR mishoo/UglifyJS2#962
  • [x] mishoo/UglifyJS2#1001
  • [x] OpenUserJS metadata block option to denote possible unstable with minification but still allow... Change UI coloring and notice of "The script author suggests that minification of this script may be unstable."
  • [ ] Migrate to terser
  • [ ] #793

SummerWish f.k.a Breeswish was gracious enough to grant the request for enhancement and a personal thank you goes out. avdg fixed the newline issue in the UserScript metadata block with minification.... once again thank you too.


OUJS refs:

  • https://openuserjs.org/discuss/How_to_disable_install_with_minification_on_my_script

Martii avatar Nov 20 '14 05:11 Martii

Before I get too gung ho on this... should there be an opt out in user preferences when logged in? or should we always minify on production (for user.js installs and what about the ../src routes?)?

Martii avatar Nov 20 '14 08:11 Martii

Never minify the user.js. None of the other sites minify by default.

End users should not have to deal with minification.

Zren avatar Nov 20 '14 08:11 Zren

Well we should be the first to offer a feature that beats out the other run of the mill sites. ;) And that's not entirely true... gf, in a similar fashion, has discussed default languages via whatever persistent storage... which this can be considered inclusive.

And yes end users should be dealing with portable device constraints. JavaScript white space can take around 20% to 30% of mass storage which is really just wasted space (this was proven with CI btw on USO).

The only reason I offer a question here is because if someone wants to back up their unminified code (say they aren't on GH for some reason) they can... but the ../src route could not minify.

See also:

  • #116 especially sizzles responses.

Martii avatar Nov 20 '14 08:11 Martii

first to offer a feature

Minification is not a feature for end users. It's a feature to lower bandwidth for the server owner. One of our rules is to not minify your scripts, why in tarnation would we then minify it for the user so he can't read/edit the source in his GM addon?

We could use gzip compression to transfer it to the user if bandwidth is the issue.

Zren avatar Nov 20 '14 09:11 Zren

One of our rules is to not minify your scripts

No it's not... see https://openuserjs.org/about/Terms-of-Service#minification

We could use gzip compression to transfer it to the user if bandwidth is the issue.

We already do... see https://github.com/OpenUserJs/OpenUserJS.org/blob/f3233decd7368aeb1fa425ffbd4a24abc8395fb0/app.js#L7 and https://github.com/OpenUserJs/OpenUserJS.org/blob/f3233decd7368aeb1fa425ffbd4a24abc8395fb0/app.js#L68

why in tarnation would we then minify it for the user so he can't read/edit the source in his GM addon?

...

should there be an opt out in user preferences when logged in?

Can offer an opt in too... I am just handing the conch around with this question.

Btw haven't seen "tarnation" in a looooooong time. ;)

Martii avatar Nov 20 '14 09:11 Martii

should there be an opt out in user preferences when logged in?

No because you assume the person will always have a logged in session, which they won't. You assume the GM addon will even send the cookies to signal there's a session, which I'm not sure about.

Er wait... who has the opt in option? An end user (see issues above) or a script auther (see issues in previous comment).

Zren avatar Nov 20 '14 09:11 Zren

who has the opt in option?

I don't always follow to conclusion every greasyfork (gf) issue but it was under discussion last time I read for default languages (i18n or whatever silly name someone wants to use).

No because you assume the person will always have a logged in session, which they won't. You assume the GM addon will even send the cookies to signal there's a session, which I'm not sure about.

And because I maintain the GM Port on SF I know exactly what GM will do and not do currently... updating on a portable device will most likely not be logged in to conserve their bandwidth $$$ not ours... so logged out would either need something persistent in the user.js engine/browser or just default to on... which leaves logged in for opt out. I've had many months (technically years before the divestiture of USO) to think about this even before gf thought about it for translations ... but I wanted to hear some feedback before I go "gung ho" on this.

Martii avatar Nov 20 '14 09:11 Martii

I do want you to cross-examine a similarity between your quotes here and notice something:

why in tarnation would we then minify it for the user so he can't read/edit the source in his GM addon?

...

Except chrome (and probably firefox) can unminify/prettyprint it for you.

Martii avatar Nov 20 '14 09:11 Martii

updating on a portable device will most likely not be logged in to conserve their bandwidth $$$ not ours...

You could check UserAgents but that would be a bitch to keep track of. It would also make testing a pain in the ass. Also, they are not downloading unless there's an update. The GM addon should prompt before downloading over their data plan. We're also talking about files < 1 Mb here... If that's costing you 1% of your data plan, then get on wireless beforehand ffs.

I do want you to cross-examine a similarity between your quotes here and notice something:

One comment is about something the end user see (userscripts). The other is about shit the developer sees (who has tools to unminify). Also, most GM addons editors are a POS (textareas). Users should not have to unminify to read/edit a userscript.

Zren avatar Nov 20 '14 09:11 Zren

An end user (see issues above) or a script auther (see issues in previous comment).

And yes that is another valid question... which I missed the first read around above... I thought of that too... serving minified by author choice could be an option too.

Martii avatar Nov 20 '14 09:11 Martii

Users should not have to unminify to read/edit a userscript.

That's what the Source Code tab is for on OUJS and GH commits/trees/heads/branches/ etc. I didn't say minify it there... although some authors will probably still want to "game" the current 1MiB limit.

Martii avatar Nov 20 '14 09:11 Martii

That's what the Source Code tab is for on OUJS and GH commits/trees/heads/branches/ etc. I didn't say minify it there... although some authors will probably still want to "game" the current 1MiB limit.

If a user provides a script without different translations (and also does not support easy i18n) users often translate the strings of the script in the downloaded one. Also if there are bugs in the script I often check my own installed code and modify it prior to check the code on the page. So this should not be forced! What about having a second install button which says "Install for development" or only "Development" on it where you can get the unmodified version instead of providing a opt in for all pages? So the normal install is minified and developers have a second install option which is non minified?

Also I can not remeber such a discussion on GF.

tobbexiv avatar Nov 20 '14 19:11 tobbexiv

What about having a second install button which says "Install for development" or only "Development" on it where you can get the unmodified version instead of providing a opt in for all pages?

A selective install button is also an option with a unique route and/or QSP... I was thinking about toying with a drop down menu type similar to installWith (if you have ever used that when USO was around) but of course with bootstraps CSS instead. e.g. http://getbootstrap.com/components/#dropdowns

A few things need to be retested here to ensure that it's still compatible to do all of this which of course can be in the dev environment.

Thank you for your feedback tobias-engelmann and great to see a newer returning voice. :)


Thinking out loud here for route possibilities (and @tobias-engelmann do any of these seem appealing?):

/install/author/*.min.user.js
/src/libs/author/*.min.js
/src/scripts/author/*.min.user.js
/minstall/author/*.user.js
/msrc/libs/author/*.js
/msrc/scripts/author/*.user.js
/installmin/author/*.user.js
/srcmin/libs/author/*.js
/srcmin/scripts/author/*.user.js

Martii avatar Nov 21 '14 00:11 Martii

I'm against minifying the userscripts. As noted before I'm against modifying my userscripts in anyway, and minification is one modification.

The reason is that my scripts are tested when I write them and minifying alters the script after they are published. Another reason as an advantaged user, is that modifying a script is a whole lot harder when minified.

I do see advantages for users that are on their mobile devices (where bandwidth is an issue) and want to install the smallest possible. I prefer a solution that let's the user choose. The solution I would suggest is to put a (smaller) button besides the install button stating that you download the same script for mobile.

jerone avatar Nov 21 '14 09:11 jerone

Is it even worth minifying when we have compression? Compressing ASCII already shrinks responses by a lot I would think. I would think that it would shrink whitespace pretty easy as well as repeated characters compresses well. Decided to do some tests.

Test source: https://openuserjs.org/scripts/nobodyrandom/MouseHunt_AutoBot_REVAMP Reuploaded to: https://openuserjs.org/scripts/Zren/TestMinification And minified here: https://openuserjs.org/scripts/Zren/TestMinification.min

A 100kb script unminified gets compressed to ~15-20Kb. Minified (without comments, so the real thing would be bigger) it's 50kb. Compressed it's 9kb.

This would save approximately 5Kb per script update... That's not even 1 webpage's worth of bandwidth savings.

It's not like we're transferring movies here people...

Zren avatar Nov 21 '14 09:11 Zren

@Zren commented on 21 nov. 2014 10:57 CET:

It's not like we're transferring movies here people...

Great test. It's worth noting that your test script has around 2300 lines of code (including comments), which is a pretty decent sized userscript. To my opinion the advantages (reduced bandwidth) of minifying are too small to overrule the users ability to offline edit userscripts and other.

jerone avatar Nov 21 '14 11:11 jerone

I prefer a solution that let's the user choose.

Agreed... opt in... which is why installWith was always opt in and I did allude to it at https://github.com/OpenUserJs/OpenUserJS.org/issues/116#issuecomment-44769613.

I'll fidget around with the UI first but I need some more current stats based of OUJS values instead of a single unit test that has just been presented... I may have to make Count Issues work with userscripts-mirror.org to pull up a more accurate test base. Some dependencies also depend on across the pond with this as well... as it sits now my tracking upstream is potentially blocking.

Martii avatar Nov 21 '14 16:11 Martii

And I'm going to say this differently and don't pull it out context in the future... I don't care about our server bandwidth for this issue... it's primarily for storage on portable devices and if everyone rereads https://github.com/OpenUserJs/OpenUserJS.org/issues/116#issuecomment-44769355 you might get some more insight... since I'm the one that has been administrating OUJS production primarily more than anyone else (hint, hint) it would be nice to have this ability.

Martii avatar Nov 21 '14 16:11 Martii

Here's some stats on one single lonely, active, Unit Test script that appears to be "wildly" popular ;)

Script Full Basic available minified
oujs - Meta View 2.2.10¹ 17324 bytes 12660 bytes

So let's see...

  • 100% script size on full.
  • ~26.9% reduction of mass storage for basic minified. (the approximate tilde is to make this apples to apples because it got slightly bigger with the @name change and current mantissa precision)

... and that's just one script that is very tiny.

Cluster/block size will always be an issue on some platforms and I had this argument with an ex supervisor back in the early 90's... and after 2 years of that debate I upgraded to FAT32 on that machine and all the sudden we didn't have to buy tiny HDDs all the time (which at the time were very pricey and costing the corporation beau coup bucks) and have one or more full SCSI array farms. e.g. he lost the debate and I also was offered his job. I understand that you all are trying to grasp this concept and I hope that you can do some further reading and learn from real world, practical, verified experience.

Martii avatar Nov 21 '14 17:11 Martii

I would suggest is to put a (smaller) button besides the install button stating that you download the same script for mobile.

There's also more than one level of minification and there's also obfuscation possibilities... so a drop down is better suited for scaling. The default should be raw, author initiated, as I have stated since at least early 2010 with installWith. I asked the question to get your opinions since quite a few of you actually ignored it in #116 ... e.g. you all were sleeping and/or possibly slacking for approximately six months or longer... but at least some of you are here now.

Martii avatar Nov 21 '14 18:11 Martii

Test source: https://openuserjs.org/scripts/nobodyrandom/MouseHunt_AutoBot_REVAMP Reuploaded to: https://openuserjs.org/scripts/Zren/TestMinification And minified here: https://openuserjs.org/scripts/Zren/TestMinification.min

A 100kb script unminified gets compressed to ~15-20Kb. Minified (without comments, so the real thing would be bigger) it's 50kb. Compressed it's 9kb.

Here's what those two scripts of yours compute here:

  • https://openuserjs.org/scripts/Zren/TestMinification at 111362 bytes
  • https://openuserjs.org/scripts/Zren/TestMinification.min at 54967 bytes

111362 B - 54967 B = 56395 B approximately == 108.75KiB - 53.68KiB = 55.07KiB

What is the difference here again and percentage? (rhetorical and granted you stripped all comments out except the metadata block which is why I said there is different levels of minification and that's one of them)


$ ls -al
...
-rw-rw-r--  1 user user  54967 Nov 21 14:45 TestMinification.min.user.js
-rw-rw-r--  1 user user 111362 Nov 21 14:44 TestMinification.user.js

$ sudo tune2fs -l /dev/sda5
tune2fs 1.42.5 (29-Jul-2012)
...
Block size:               4096
...

Martii avatar Nov 21 '14 19:11 Martii

I remember my families first hard drive. It was 89Mb big. I managed to fill it up with super awesome MS paint drawings. Fun times. That was back when Win 3.1 was around. So over a decade ago. Storage isn't measured in Kilobytes or event megabytes anymore. It's measured in hundreds of gigabytes to terabytes. Even phone have at least a gigabyte or two of storage. I can't imagine a user using up more than 10Mb to store all their userscripts on their phone. Minification saves (in our two test cases) 30-50% storage, an upwards of 5Mb saved.

That said. Storage of userscripts clientside should be handled and solved by the browser addon. Should storage be an issue, they will probably compress the scripts, which is a lossless operation. If they use the same algorithm we use when transferring the script, it compresses to ~20% of the original size.

Marti: Regardless of whether it could be implemented it would be encouraging open source rather than encouraging authors to minify/obfuscate. It would also keep the moderators and up from getting burnt out too quickly.

Sizzle: This is good reason to implement minifying on the site. If we do it for them, maybe they'll let us get a look at the original source. I'd also give us good cause to ban minified scripts on the site since there would no longer be a valid reason to upload them.

If moderation is an issue with minified scripts, why not make a button that pretty prints the source code on the source page. It'd be a client side script to lessen the burden on computational resources.

Also should point out that a user might want to minify a script to get it under our script size limit. Though the only script that's pushing over 1Mb is probably YT Center atm.

To summarize my points.

Pros for minification

  • Smaller bandwidth: After compression, the difference is about 5kb less, which isn't worth bothering with for mobile data bandwidth. Server bandwidth is not an issue.
  • Smaller storage clientside: Storing text does not eat up much space. If it is a concern, it is the job of the GM addon to compress the script.
  • Moderation: We do not need to offer a minification feature in order to ban user submissions with minification. We can provide an unminify source code button to make it easier to moderate minified scripts.

Cons for minification

  • Harder to read/edit in the GM addon.
  • Multiple install buttons make installing more complicated. While we definitely don't want to go down the road of UserScripts.org's second install button in the ads, we will make curious users who click the dropdown consider "Which version should I pick?".
  • We modify the authors script.

Zren avatar Nov 21 '14 22:11 Zren

If moderation is an issue with minified scripts, why not make a button that pretty prints the source code on the source page

Already have that user.js side and why do you think I've been fidgeting with Ace more and more... e.g. node and the packages we utilize (and researched the ones we don't) aren't quite there yet. You might consider following my RSS feed to be up to date on this.


Also should point out that a user might want to minify a script to get it under our script size limit. Though the only script that's pushing over 1Mb is probably YT Center atm.

same as..

although some authors will probably still want to "game" the current 1MiB limit.


Anyhow without reiterating a dead horse here... the consensus and establishing owner approval is currently at:

  • +1 sizzle with positive acknowledgement
  • 0+ jerone with stipulation that it should be optional
  • -1 zren … you know your motivations
  • +1 me
  • +1 tobias with stipulation that it not interfere with local editing.

The question is closed and it's moving forward with 6 months plus time already given for discussion... again I can't say when because there is a bug right now but I may be creating the route for it in experimental status so it can be tracked and tested on pro in case some other weird bug shows up... We might even need to wait for node 0.11.x too for official support since ES6 isn't supported by most parsers and it won't minify as of last night... thought that was another interesting pitfall of V8.


P.S. 8 inch diskettes rule ;)


If anyone is willing to participate in the experimental phase the preferred route so far for installs has been suggested as:

/min/author/scriptname.user.js

... this offers the least impact to everyone... don't know about the other two existing routes for non-counted installs yet.

or possibly...

/install/min/author/scriptname.user.js
/src/min/libs/author/scriptname.js
/src/min/scripts/author/scriptname.user.js

... but I think I can see already an issue there on the latter... tinkering time first though.

Martii avatar Nov 21 '14 23:11 Martii

btw this falls a little under #135 and #249 which is also a little of #262 ... and since GH can't do multiple milestones... about time to create #135 milestone.

Martii avatar Nov 21 '14 23:11 Martii

Now that this has been in experimental status for a few weeks _(and there is interest)_ I wanted to reiterate in some different words that minification is a type of "site forking" in which OUJS via UglifyJS2 forks the script in minified form on installation. Being a little direct here for prior remarks... forking is allowed on any script... this is no different than GH... just a little more automated. Authors may at their option "unsupport" this if issues are found with minfied code but that becomes an upstream issue with UglifyJS2 as we at OUJS are a pass through for this. e.g. a messenger. If express-minify encounters an error with minification it has an error trap to return the native/raw code.

Since SummerWish is a little busy at the moment I've temporarily forked his project on OUJS and bumped the version for testing on a branch. All of my ES6 scripts minify now in dev and local pro... however those are also ancient from the defunct USO. jerone has one ES6 stated specific script and when I bump the dep to our fork he and his users can say if it's working before an update from GH. (this last part will lead into some discussion as well but one step at a time)

There are a few more options that we can set, including but not limited to, not mangling identifier names e.g. var someIdentifier = 10; becomes shortened with some other letter (back to BASIC days I guess) such as var a = 10;... this is one of the defaults we're currently using already with ES5. Currently we use all the defaults of UglifyJS2 via express-minify.

As a final note... we've been minifying all of our deps .js since express-minify was added... so it does work well so far.

Martii avatar Jan 19 '16 06:01 Martii

I'm not sure why, but... Try this one: https://openuserjs.org/scripts/ts/Test_Script_for_Keep_Comments It removed the comment 8

tiansh avatar Jan 21 '16 02:01 tiansh

@tiansh

It removed the comment 8

Try it on a newline and see what it does... there was a newline issue that may affect this... I don't know if all types were affected... // vs /*


btw CC 0 isn't allowed on licensing... that's considered to be a type of public domain by OSI.

Martii avatar Jan 21 '16 02:01 Martii

@Martii ahh, i got it. it is an old issue on uglifyjs...

tiansh avatar Jan 21 '16 03:01 tiansh

@tiansh Cc: @mishoo , @avdg , @fabiosantoscode , @SummerWish

So I've retested a few tests... and yes Function Expressions have some issues currently... which is unfortunate. I've disabled all of the compressor options in another test and it still wipes out the comments in certain situations.

As we initially stated make a comment on your script homepage for now... eventually I'll attempt to put in a key for the OpenUserJS metadata block that will be something like @unstableMinify or the like if an author ends up using those... until then "don't bend it where it hurts" is a good idea. I want to give UglifyJS2 every chance to contribute and test their ideas... so disabling this feature is not an option at this time... just like disabling OUJS support issues isn't an option even if someone is using another issue tracker like GH with @support.

The ES6 template strings @sizzlemctwizzle mentioned might be a better route for coding in general. I have not tested those yet with this feature as the harmony branch hasn't been activated on OUJS yet and is only local in my dev right now. UPDATE: Tested with this... does work however not full browser compliment support.

Please remember this feature enhancement is still experimental and when I get to the point where I can dig into UglifyJS2 a bit I might be able to add a new perspective but I'll have to learn their system more. I'm still just barely working/learning with the current API that has been exposed.

With time things can change... and I know some people are afraid of change.

Ref:

  • mishoo/UglifyJS2#88

Martii avatar Jan 21 '16 07:01 Martii

@tiansh Cc: @jesus2099 (some of your UserScript metadata block wraps apply to this as well ex. here)

I have roughed in the feature in for your edge use case. I would suggest using something like this OpenUserJS metadata block template for what you need... this is now active on production. Please note this is not inclusive to the UserScript metadata block.

As I mentioned on OUJS production putting in a reason improves your users experience as to why something may or may not be useful to them... plus we can track down if the dependency fails with some good explanations... so I recommend something like the below message:

// ==OpenUserJS==
// @unstableMinify Multi-line string comments using Function Expressions
// ==/OpenUserJS==

General format is // @unstableMinify some brief reason string and only one, last, key is currently used as per GM specifications, in the OpenUserJS metadata block. e.g. most people won't need a novel.


Currently used in the wild:

Martii avatar Feb 05 '16 02:02 Martii