Fix minified dist bundles #1094
Conversation
- bumping to node v6 disrupted the "a(r)" assumption
- so that both bundle.js and cibundle.js can use it.
- so that require.js test can be properly ran on each build!
| var strOld = FRONT + MIDDLE + BACK, | ||
| strNew = FRONT + ' ' + MIDDLE + ' ' + BACK; | ||
|
|
||
| minifiedCode = minifiedCode.replace(strOld, strNew); |
There was a problem hiding this comment.
if there's a way to do using Regex, please let me know!
There was a problem hiding this comment.
@rreusser 's got the answer
minifiedCod.replace(/require\("\+(\w)\((\w)\)\+"\)/, 'require("+ $1($2) +")')
There was a problem hiding this comment.
It could also be done using .split(strOld).join(strNew) but I have no idea if it would be any faster in this case.
There was a problem hiding this comment.
This is a great tool for these things:
There was a problem hiding this comment.
A slight modification for lower or uppercase:
/require\("\+([A-z])\(([A-z])\)\+"\)/There was a problem hiding this comment.
I counter with https://regex101.com/
| } | ||
| } | ||
|
|
||
| return minifiedCode; |
There was a problem hiding this comment.
This is hilarious. Do we need to check for uppercase letters or does it stick to all lowers?
There was a problem hiding this comment.
Good call. Does uglify-js use uppercase lettes? I don't know. But anyway, @rreusser's solution should cover that case too.
There was a problem hiding this comment.
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?
| * | ||
| */ | ||
| module.exports = function patchMinified(minifiedCode) { | ||
| return minifiedCode.replace(STR_TO_REPLACE, STR_NEW); | ||
| return minifiedCode.replace(PATTERN, NEW_SUBSTR); |
The ugly
webworkify+ Require.js bug strikes again 👻 The1.19.0dist builds are broken in Require.js. In #914 (more specifically #914 (comment)) I chose to run the plotly.min.js + require.js test only on the main dist bundle because minifying plotly.js takes about 1 minute. That is, it was testing new code only on release builds (like this one).BUT, I should add: minifying plotly.js take about 1 minute, that's using node.js v4. Using node.js v6, the minifying time drops to about ~30 seconds. Good enough for me. Let's test this thing on every build!