Skip to content

[SE-3401] Updates JakePackage.zip to make JS minification consistent across different machines - #275

Merged
nizarmah merged 4 commits into
opencraft-release/juniper.3from
nizar/update_tinymce_jakeconfig_juniper_3_backport
Oct 18, 2020
Merged

[SE-3401] Updates JakePackage.zip to make JS minification consistent across different machines#275
nizarmah merged 4 commits into
opencraft-release/juniper.3from
nizar/update_tinymce_jakeconfig_juniper_3_backport

Conversation

@nizarmah

@nizarmah nizarmah commented Oct 10, 2020

Copy link
Copy Markdown

When generating the minified files for the TinyMCE static files using the vendor_extra/tinymce/JakePackage.zip, there's often a problem that was happening where the files are different after running the following command:

jake minify bundle[themes:modern,plugins:advlist,anchor,autolink,charmap,code,codemirror,contextmenu,image,insertdatetime,link,lists,media,paste,print,save,searchreplace,table,textcolor,visualblocks]

This becomes an issue when installing a new TinyMCE plugin on different machines and re-running jake minify bundle, because the files are not consistent on different machines. This results in a different commons.js file hash on the different machines, which makes it not possible to use multiple instances with a load balancer.

Some of the issues are, amdlc compiles the minified output for a plugin, and then uglify-js doesn't update the file. Therefore, removed the minification process from the amdlc step and reverted minification to uglify-js.

This PR simply adds a specific configuration for uglify-js, which is:

mangle : {
	sort: true,
	toplevel: false
},
compress: {
	cascade: false,
	comparisons: false,
	dead_code: true,
	if_return: true
},
output: {
	ascii_only: true,
	beautify: false
},
toplevel: false

One of the most important is, sort which makes the mangled variable names consistent on different runs. Also, comparisons and if_return are specified because they were sometimes applied, while others times, they weren't.

JIRA tickets: SE-3401, SE-3101, SE-3381

Upstream PR: edx#25324

Testing instructions:

  1. Change your directory to common/static/js/vendor/tinymce/
  2. Once you are there, run unzip ../../../../../vendor_extra/tinymce/JakePackage.zip
  3. Clean the existing npm modules, rm -rf node_modules
  4. Install the npm dependencies, npm install
  5. Run the jake clean command, npx jake clean
  6. Run the jake command, mentioned below.
  7. Commit the changes since I didn't update the existing minified files.
  8. Run the jake command, mentioned below, again.
  9. Make sure no changes have been made to the files.
  10. Verify all plugin.min.js were generated correctly using: grep -Er "^undefined$".
npx jake minify bundle[themes:modern,plugins:advlist,anchor,autolink,charmap,code,codemirror,contextmenu,image,insertdatetime,link,lists,media,paste,print,save,searchreplace,table,textcolor,visualblocks]

Author notes and concerns:

  1. Do you think it is better to update the minified files and commit those changes? Please let me know.

Files changed:

package.json:

        "repository": {
-               "type" : "git",
-               "url" : "https://github.com/tinymce/tinymce.git"
+               "type": "git",
+               "url": "https://github.com/tinymce/tinymce.git"
        },
        "description": "TinyMCE rich text editor",
        "author": "Johan Sörlin <spocke@moxiecode.com>",
-       "bugs": { "url" : "http://www.tinymce.com/develop/bugtracker.php" },
+       "bugs": {
+               "url": "http://www.tinymce.com/develop/bugtracker.php"
+       },
        "private": true,
        "engines": {
-               "node" : ">=0.10.26"
+               "node": ">=0.10.26"
        },
        "devDependencies": {
-               "jake": ">= 0.7.0",
-               "amdlc": ">= 0.0.2",
-               "jshint": ">= 2.1.4",
-               "eslint": ">= 0.4.2",
-               "uglify-js": ">= 2.0.0",
-               "glob": ">= 3.1.12",
-               "moxie-zip": ">= 0.0.1",
-               "less": ">= 1.3.1",
-               "coverjs": ">= 0.0.14"
+               "jake": "0.7.10",
+               "amdlc": "0.1.2",
+               "jshint": "2.4.4",
+               "eslint": "0.4.5",
+               "uglify-js": "2.4.13",
+               "glob": "3.2.9",
+               "moxie-zip": "0.0.3",
+               "less": "1.7.0",
+               "coverjs": "0.0.14"
        },

package-lock.json

+ everything...

Jakefile.js

@@ -189,7 +189,6 @@ task("minify-pasteplugin", [], function() {
                baseDir: "js/tinymce/plugins/paste/classes",
                rootNS: "tinymce.pasteplugin",
                outputSource: "js/tinymce/plugins/paste/plugin.js",
-               outputMinified: "js/tinymce/plugins/paste/plugin.min.js",
                outputDev: "js/tinymce/plugins/paste/plugin.dev.js",
                verbose: false,
                expose: "public",
@@ -203,7 +202,6 @@ task("minify-spellcheckerplugin", [], function() {
         baseDir: "js/tinymce/plugins/spellchecker/classes",
         rootNS: "tinymce.spellcheckerplugin",
         outputSource: "js/tinymce/plugins/spellchecker/plugin.js",
-        outputMinified: "js/tinymce/plugins/spellchecker/plugin.min.js",
         outputDev: "js/tinymce/plugins/spellchecker/plugin.dev.js",
         verbose: false,
         expose: "public",
@@ -217,7 +215,6 @@ task("minify-tableplugin", [], function() {
         baseDir: "js/tinymce/plugins/table/classes",
         rootNS: "tinymce.tableplugin",
         outputSource: "js/tinymce/plugins/table/plugin.js",
-        outputMinified: "js/tinymce/plugins/table/plugin.min.js",
         outputDev: "js/tinymce/plugins/table/plugin.dev.js",
         verbose: false,
         expose: "public",
@@ -663,3 +660,20 @@ task("clean", [], function () {
        });
 });
 
+desc("Cleans the minified JavaScript for plugins and themes");
+task("clean-js", [], function () {
+       [
+               "tmp/*",
+               "js/tinymce/tinymce*",
+               "js/tinymce/*.min.js",
+               "js/tinymce/*.dev.js",
+               "js/tinymce/plugins/table/plugin.js",
+               "js/tinymce/plugins/!(image|media)/plugin.min.js",
+               "js/tinymce/themes/*/theme.min.js",
+       ].forEach(function(pattern) {
+               glob.sync(pattern).forEach(function(filePath) {
+                       fs.unlinkSync(filePath);
+               });
+       });
+});
+

tools/BuildTools.js

@@ -31,12 +31,23 @@ exports.uglify = function(options) {
        var UglifyJS = require("uglify-js");
        var filePaths = [];
 
-       options = extend({
-               mangle : true,
-               toplevel : false,
-               no_functions : false,
-               ascii_only: true
-       }, options);
+       var uglifyOptions = {
+               mangle : {
+                       sort: true,
+                       toplevel: false
+               },
+               compress: {
+                       cascade: false,
+                       comparisons: false,
+                       dead_code: true,
+                       if_return: true
+               },
+               output: {
+                       ascii_only: true,
+                       beautify: false
+               },
+               toplevel: false
+       };
 
        var toFileModTime = getFileModTime(options.to);
        var fromFileModTime = 0;
@@ -58,8 +69,7 @@ exports.uglify = function(options) {
        }
 
        if (options.force === true || fromFileModTime !== toFileModTime) {
-               var result = UglifyJS.minify(filePaths, {
-               });
+               var result = UglifyJS.minify(filePaths, uglifyOptions);
 
                fs.writeFileSync(options.to, result.code);
                setFileModTime(options.to, fromFileModTime);

Reviewers

@nizarmah
nizarmah requested a review from pkulkark October 10, 2020 14:37
@nizarmah nizarmah self-assigned this Oct 10, 2020
@nizarmah
nizarmah force-pushed the nizar/update_tinymce_jakeconfig_juniper_3_backport branch from e653767 to 1615ce9 Compare October 11, 2020 11:35
@nizarmah nizarmah changed the title [SE-3401] Updates BuildTools UglifyJS configuration to be consistent on different machines [SE-3401] Updates BuildTools UglifyJS options and Clean task in Jakefile Oct 11, 2020
@nizarmah
nizarmah marked this pull request as draft October 11, 2020 22:43
@nizarmah
nizarmah force-pushed the nizar/update_tinymce_jakeconfig_juniper_3_backport branch 2 times, most recently from 7daf6a6 to 4b62249 Compare October 12, 2020 02:24
Jake clean is updated in order to not delete the plugin.min.js for 'image' and 'media' plugins because these plugins do not have a 'plugin.js' to create the minified file from.

Also removes 'outputMinified' for the 'minify' tasks that are included with the 'minify-plugin' so that the minification is done through uglify instead (to keep things uniform and consistent).
@nizarmah
nizarmah marked this pull request as ready for review October 12, 2020 02:28
@nizarmah nizarmah changed the title [SE-3401] Updates BuildTools UglifyJS options and Clean task in Jakefile [SE-3401] Updates JakePackage.zip to make JS minification consistent across different machines Oct 12, 2020
@nizarmah
nizarmah force-pushed the nizar/update_tinymce_jakeconfig_juniper_3_backport branch from 6a19271 to c4f2bfd Compare October 13, 2020 11:55

@pkulkark pkulkark left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nizarmah LGTM 👍

  • I didn't test this but I verified that the common.js hashes on the STG appservers are the same.
  • I read through the code
  • I checked for accessibility N/A
  • Includes documentation N/A

@nizarmah
nizarmah merged commit ac7a2a0 into opencraft-release/juniper.3 Oct 18, 2020
@nizarmah
nizarmah deleted the nizar/update_tinymce_jakeconfig_juniper_3_backport branch October 18, 2020 08:56
nizarmah added a commit that referenced this pull request Oct 18, 2020
…across different machines (#275)

* Updates jake Clean task and BuildTools Uglify options

Jake clean is updated in order to not delete the plugin.min.js for 'image' and 'media' plugins because these plugins do not have a 'plugin.js' to create the minified file from.

Also removes 'outputMinified' for the 'minify' tasks that are included with the 'minify-plugin' so that the minification is done through uglify instead (to keep things uniform and consistent).

* Specifies Version of Dependencies and Adds package-lock.json for easy Dependencies Installation

* Cleans TinyMCE minified themes javascript files to consistently re-minify them

* Adds a new task to Jakefile to clean JS only
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants