Skip to content

Commit

Permalink
core(minification): properly handle regex character classes (#6745)
Browse files Browse the repository at this point in the history
  • Loading branch information
patrickhulce committed Dec 7, 2018
1 parent 9bafaa5 commit 95b084b
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 3 deletions.
9 changes: 8 additions & 1 deletion lighthouse-core/lib/minification-estimator.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ function computeTokenLength(content, features) {
let isInLicenseComment = false;
let isInString = false;
let isInRegex = false;
let isInRegexCharacterClass = false;
let stringOpenChar = null;

for (let i = 0; i < content.length; i++) {
Expand Down Expand Up @@ -94,7 +95,13 @@ function computeTokenLength(content, features) {
// Skip over any escaped characters
totalTokenLength++;
i++;
} else if (char === '/') {
} else if (char === '[') {
// Register that we're entering a character class so we don't leave the regex prematurely
isInRegexCharacterClass = true;
} else if (char === ']' && isInRegexCharacterClass) {
// Register that we're exiting the character class
isInRegexCharacterClass = false;
} else if (char === '/' && !isInRegexCharacterClass) {
// End the string when we hit the regex close character
isInRegex = false;
// console.log(i, 'leaving regex', char)
Expand Down
29 changes: 27 additions & 2 deletions lighthouse-core/test/lib/minification-estimator-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,31 @@ describe('minification estimator', () => {
assert.equal(computeJSTokenLength(js), 9);
});

it('should handle regular expression character classes', () => {
// test a slash inside of a character class to make sure it doesn't end the regex
// The below is the string-equivalent of
const _ = /regex [^/]\//.test('this should be in string not comment 123456789');

const js = `
/regex [^/]\\//.test('this should be in string not comment 123456789')
`;

assert.equal(computeJSTokenLength(js), 69);
assert.equal(computeJSTokenLength(js), js.trim().length);
});

it('should handle escaped regular expression characters', () => {
// test an escaped [ to make sure we can still close regexes
// This is the string-equivalent of
const _ = /regex \[/; // this should be in comment not string 123456789

const js = `
/regex \\[/ // this should be in comment not string 123456789
`;

assert.equal(computeJSTokenLength(js), 10);
});

it('should distinguish regex from divide', () => {
const js = `
return 1 / 2 // hello
Expand All @@ -190,8 +215,8 @@ describe('minification estimator', () => {

it('should handle large, real javscript files', () => {
assert.equal(angularFullScript.length, 1364217);
// 1 - 405199 / 1364217 = estimated 70% smaller minified
assert.equal(computeJSTokenLength(angularFullScript), 405199);
// 1 - 334968 / 1364217 = estimated 75% smaller minified
assert.equal(computeJSTokenLength(angularFullScript), 334968);
});
});
});

0 comments on commit 95b084b

Please sign in to comment.