Skip to content

Fix minified dist bundles #1094

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Oct 27, 2016
Merged

Fix minified dist bundles #1094

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2acb387
tasks: generalise patch minified task
etpinard Oct 27, 2016
e16d2f7
tasks: factor out _bundle wrapper in own tasks/util/ module
etpinard Oct 27, 2016
d4f216f
tasks: make cibundle output plotly.min.js
etpinard Oct 27, 2016
f958b4f
doc: improve comment in patch minified tasks util
etpinard Oct 27, 2016
1e98161
tasks: use regex instead of loop over alphabet in patch minified step
etpinard Oct 27, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions tasks/util/patch_minified.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
var STR_TO_REPLACE = 'require("+a(r)+");';
var STR_NEW = 'require("+ a(r) +");';
var ALPHABET = 'abcdefghijklmnopqrstuvwxyz'.split('');
var FRONT = 'require("+';
var BACK = '+");';

/* Uber hacky in-house fix to
*
Expand All @@ -11,5 +12,20 @@ var STR_NEW = 'require("+ a(r) +");';
*
*/
module.exports = function patchMinified(minifiedCode) {
return minifiedCode.replace(STR_TO_REPLACE, STR_NEW);
for(var i = 0; i < ALPHABET.length; i++) {
var li = ALPHABET[i];

for(var j = 0; j < ALPHABET.length; j++) {
var lj = ALPHABET[j];

var MIDDLE = li + '(' + lj + ')';

var strOld = FRONT + MIDDLE + BACK,
strNew = FRONT + ' ' + MIDDLE + ' ' + BACK;

minifiedCode = minifiedCode.replace(strOld, strNew);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if there's a way to do using Regex, please let me know!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rreusser 's got the answer

minifiedCod.replace(/require\("\+(\w)\((\w)\)\+"\)/, 'require("+ $1($2) +")')

Copy link
Contributor

Choose a reason for hiding this comment

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

It could also be done using .split(strOld).join(strNew) but I have no idea if it would be any faster in this case.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a great tool for these things:

http://scriptular.com/#require%5C(%22%5C%2B(%5Cw)%5C((%5Cw)%5C)%5C%2B%22%5C)%7C%7C%7C%7C%7C%7C%7C%7C%5B%22require(%5C%22%2Bo(s)%2B%5C%22)%22%5D

Copy link
Contributor

@rreusser rreusser Oct 27, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

A slight modification for lower or uppercase:

/require\("\+([A-z])\(([A-z])\)\+"\)/

Copy link
Contributor

Choose a reason for hiding this comment

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

I counter with https://regex101.com/

}
}

return minifiedCode;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is hilarious. Do we need to check for uppercase letters or does it stick to all lowers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. Does uglify-js use uppercase lettes? I don't know. But anyway, @rreusser's solution should cover that case too.

Copy link
Contributor

@rreusser rreusser Oct 27, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I always forget the semantics of A-z vs. \W vs. \w. Okay:

  • \w: Matches any alphanumeric character including the underscore. Equivalent to [A-Za-z0-9_].
  • \W: Matches any non-word character. Equivalent to [^A-Za-z0-9_]. (= negation)
  • [A-Za-z] any lower or uppercase character
  • [A-z] maybe the same?

};