-
Notifications
You must be signed in to change notification settings - Fork 307
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
Test and support new option for *express-minify* with keeping comments #432
Comments
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?)? |
Never minify the user.js. None of the other sites minify by default. End users should not have to deal with minification. |
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 See also:
|
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. |
No it's not... see https://openuserjs.org/about/Terms-of-Service#minification
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
...
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. ;) |
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). |
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).
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. |
I do want you to cross-examine a similarity between your quotes here and notice something:
...
|
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.
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. |
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. |
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. |
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?):
|
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. 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. |
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 A 100kb script unminified gets compressed to ~15-20Kb. 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 commented on 21 nov. 2014 10:57 CET:
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. |
Agreed... opt in... which is why installWith was always opt in and I did allude to it at #116 (comment). 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. |
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 #116 (comment) 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. |
Here's some stats on one single lonely, active, Unit Test script that appears to be "wildly" popular ;)
So let's see...
... 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. |
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. |
Here's what those two scripts of yours compute here:
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
... |
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.
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
Cons for minification
|
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.
same as..
Anyhow without reiterating a dead horse here... the consensus and establishing owner approval is currently at:
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:
... this offers the least impact to everyone... don't know about the other two existing routes for non-counted installs yet. or possibly...
... but I think I can see already an issue there on the latter... tinkering time first though. |
* Currently is... but may not always be Applies to #432 Auto-merge
* Please read their CHANGELOGs ... *uglify-*'s may have fixed missing comments from OpenUserJS#432 * Delete op retested NOTES: * *mongoose* and *mongodb* are going through a few major breaking changes and holding off until more dedicated time can be done for migration and retesting
* Please read their CHANGELOGs ... *uglify-*'s may have fixed missing comments from #432 * Delete op retested NOTES: * *mongoose* and *mongodb* are going through a few major breaking changes and holding off until more dedicated time can be done for migration and retesting Auto-merge
* Move `console` output to dev only for experimental minification Applies to OpenUserJS#432 OpenUserJS#249 OpenUserJS#430
* Change a few classes around for UI coloring and display * Shows exclamation on script homepage and script lists that there **may** be user initiated, system initiated, etc. notices on the Install button or Raw Source button e.g. the Source Code tab contents. * Few mustache identifier name changes for symmetry Applies to OpenUserJS#1548 OpenUserJS#432
* Change a few classes around for UI coloring and display * Shows exclamation on script homepage and script lists that there **may** be user initiated, system initiated, etc. notices on the Install button or Raw Source button e.g. the Source Code tab contents. * Few mustache identifier name changes for symmetry Applies to #1548 #432 Auto-merge
* Little less prominent for warnings vs. possible critical issue * More in line with the docs that mostly say "blue install button" * Add `updateURL` check for all modes and display if present * Reorder the UI notices a bit. * Some line length conformance Post OpenUserJS#1632 and applies to OpenUserJS#1548 OpenUserJS#432
* Little less prominent for warnings vs. possible critical issue * More in line with the docs that mostly say "blue install button" * Add `updateURL` check for all modes and display if present * Reorder the UI notices a bit. * Some line length conformance Post #1632 and applies to #1548 #432 Auto-merge
* Chromium 75.0.3770.90 started spewing this out and it's not in *mime-db* dep (yet?)... Relates to `/install/<username>/<scriptname>.meta.js`. Don't think it has an extension spec based off skimming doc * Relaxing is temporary atm in lieu of more aggressive re Post OpenUserJS#1633 OpenUserJS#1632 OpenUserJS#944 and applies to OpenUserJS#1548 OpenUserJS#432
* Chromium 75.0.3770.90 started spewing this out and it's not in *mime-db* dep (yet?)... Relates to `/install/<username>/<scriptname>.meta.js`. Don't think it has an extension spec based off skimming doc * Relaxing is temporary atm in lieu of more aggressive re Post #1633 #1632 #944 and applies to #1548 #432 Auto-merge
* Some Authors are copying the trailing `#` from raw view... so give them the correct ones for .user.js engines. Also inclusive of minification routine. Post OpenUserJS#1654 and additional for OpenUserJS#432
* This was already implemented pre W3C recommendation in our form but normalizing to their syntax. * UI and DB remaining non-base64 encoded... semver limitation with extra characters that violate that spec. * Change caching mechanism... unfortunately traffic for a while will be increased while syncing with browsers. Also because spec doesn't use hex, which it probably should, the eTag header value will be bigger. Hashes, so far, are always "hex-able" by design of SHA but that could change in the future... who knows. * Base62 being dropped in favor of Base64 for cache mechanism. Should be okay with extra `+/` in base64 since that falls within ASCII limitations. * Any .user.js utilizing the .meta.json, or other language, will need to modify to check for the `sha512-` prefix and decode the value appropriately. * If .meta.json shows empty `hash` clear browser cache *(weird Fx issue perhaps)* * Bugfix on local copy of metadata script access... non-fatal atm just incorrect live copy referenced. Post OpenUserJS#1076 and applies to OpenUserJS#432 OpenUserJS#249 Ref(s): * https://developer.mozilla.org/docs/Web/HTTP/Headers/ETag * https://developer.mozilla.org/docs/Web/Security/Subresource_Integrity * https://w3c.github.io/webappsec-subresource-integrity/ * https://www.srihash.org/
* This was already implemented pre W3C recommendation in our form but normalizing to their syntax. * UI and DB remaining non-base64 encoded... semver limitation with extra characters that violate that spec. * Change caching mechanism... unfortunately traffic for a while will be increased while syncing with browsers. Also because spec doesn't use hex, which it probably should, the eTag header value will be bigger. Hashes, so far, are always "hex-able" by design of SHA but that could change in the future... who knows. * Base62 being dropped in favor of Base64 for cache mechanism. Should be okay with extra `+/` in base64 since that falls within ASCII limitations. * Any .user.js utilizing the .meta.json, or other language, will need to modify to check for the `sha512-` prefix and decode the value appropriately. * If .meta.json shows empty `hash` clear browser cache *(weird Fx issue perhaps)* * Bugfix on local copy of metadata script access... non-fatal atm just incorrect live copy referenced. Post #1076 and applies to #432 #249 Ref(s): * https://developer.mozilla.org/docs/Web/HTTP/Headers/ETag * https://developer.mozilla.org/docs/Web/Security/Subresource_Integrity * https://w3c.github.io/webappsec-subresource-integrity/ * https://www.srihash.org/ Auto-merge
* Docs say exactly "Pass false to disable minifying (JS, CSS and JSON)" which is incorrect. See archived mirror at https://github.com/OpenUserJS/express-minify/blob/f0696f6d3976b6ac7c138d767bb62ae3835ddb38/index.js#L76 . Only does JS and CSS but not JSON. * Affects admin processes and any meta.json . We already minimize with conditionals of pro vs dev and shouldn't be needed. Applies to OpenUserJS#432
* Docs say exactly "Pass false to disable minifying (JS, CSS and JSON)" which is incorrect. See archived mirror at https://github.com/OpenUserJS/express-minify/blob/f0696f6d3976b6ac7c138d767bb62ae3835ddb38/index.js#L76 . Only does JS and CSS but not JSON. * Affects admin processes and any .meta.json . We already minimize with conditionals of pro vs dev and shouldn't be needed. Applies to #432 Auto-merge
Post OpenUserJS#1076 OpenUserJS#1826 and applies to OpenUserJS#432 OpenUserJS#249 NOTE: * This increases the server load for more frequent accuracy
See also:
not released yet thoughv2.6.1+)catch
callbacks, emitters, or equivalent options breezewish/express-minify#44return
outside of function mishoo/UglifyJS#935 with PR Create and mapbare-returns
into newparse
property name mishoo/UglifyJS#962SummerWish 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:
The text was updated successfully, but these errors were encountered: