Skip to content

Callback Support #907

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

Open
RebeccaStevens wants to merge 16 commits into
base: gh-pages
Choose a base branch
from
Open

Callback Support #907

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
8a675e4
Added tests for async style and script minification using callbacks.
Apr 11, 2018
7f1dc9c
Added more tests for async style and script minification using callba…
Apr 11, 2018
8a3105a
Added a test to ensure that the minify function still results a resul…
Apr 11, 2018
a68ee19
Added callback parameter to minify.
Apr 11, 2018
892ca4b
Added callback parameter to minifyCSS and minifyJS.
Apr 11, 2018
77f3786
Added an onComplete function to the the HTMLParser class.
Apr 11, 2018
127669d
Switch out the iterative loop for a recursive loop that supports call…
Apr 11, 2018
16c9ab7
Wrapped the parse error check up into a function so its execution can…
Apr 11, 2018
9c447fd
Refactored HTMLParser's parse loop.
Apr 11, 2018
bac6a0e
Code at the end of the minify function now waits until the HTMLParser…
Apr 11, 2018
b2626ce
Added callback to chars function which will always be called once it …
Apr 11, 2018
143a8fe
HTMLParser now supplies the chars's callback function which will cont…
Apr 11, 2018
e487a50
Moved parse() call up so that it isn't hidden within function definit…
Apr 11, 2018
b51fe21
Added error handling.
Apr 11, 2018
76007a1
Fixed comment.
Apr 11, 2018
65d142d
Switch out undefined compare for typeof undefined compare.
RebeccaStevens Apr 11, 2018
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
Prev Previous commit
Next Next commit
Added error handling.
  • Loading branch information
Rebecca Stevens committed Apr 11, 2018
commit b51fe21dc29462d6ed758c13a726dde990da2ca0
55 changes: 38 additions & 17 deletions src/htmlminifier.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -942,7 +942,7 @@ function minify(value, options, partialMarkup, cb) {
}

var str;
new HTMLParser(value, {
var parser = new HTMLParser(value, {
partialMarkup: partialMarkup,
html5: options.html5,

Expand Down Expand Up @@ -1172,24 +1172,34 @@ function minify(value, options, partialMarkup, cb) {
}

if (isExecutableScript(currentTag, currentAttrs)) {
tasksWaitingFor++;
var minifyJSResult = options.minifyJS(text, null, onTaskFinished);

// If the result is defined then minifyJSResult completed synchronously.
// eslint-disable-next-line no-undefined
if (minifyJSResult !== undefined) {
onTaskFinished(minifyJSResult);
try {
tasksWaitingFor++;
var minifyJSResult = options.minifyJS(text, null, onTaskFinished);

// If the result is defined then minifyJSResult completed synchronously.
// eslint-disable-next-line no-undefined
if (minifyJSResult !== undefined) {
onTaskFinished(minifyJSResult);
}
}
catch (error) {
cb(error);
}
}

if (isStyleSheet(currentTag, currentAttrs)) {
tasksWaitingFor++;
var minifyCSSResult = options.minifyCSS(text, onTaskFinished);

// If the result is defined then minifyCSS completed synchronously.
// eslint-disable-next-line no-undefined
if (minifyCSSResult !== undefined) {
onTaskFinished(minifyCSSResult);
try {
tasksWaitingFor++;
var minifyCSSResult = options.minifyCSS(text, onTaskFinished);

// If the result is defined then minifyCSS completed synchronously.
// eslint-disable-next-line no-undefined
if (minifyCSSResult !== undefined) {
Copy link
Collaborator

@alexlamsl alexlamsl Apr 11, 2018

Choose a reason for hiding this comment

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

Please use typeof minifyCSSResult !== 'undefined' instead of overriding eslint

Copy link
Author

@RebeccaStevens RebeccaStevens Apr 11, 2018

Choose a reason for hiding this comment

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

Will do.

onTaskFinished(minifyCSSResult);
}
}
catch (error) {
cb(error);
}
}

Expand Down Expand Up @@ -1260,7 +1270,9 @@ function minify(value, options, partialMarkup, cb) {
},
customAttrAssign: options.customAttrAssign,
customAttrSurround: options.customAttrSurround
}).onComplete(function() {
});

parser.onComplete(function() {
if (options.removeOptionalTags) {
// <html> may be omitted if first thing inside is not comment
// <head> or <body> may be omitted if empty
Expand Down Expand Up @@ -1305,8 +1317,17 @@ function minify(value, options, partialMarkup, cb) {
options.log('minified in: ' + (Date.now() - t) + 'ms');

if (typeof cb === 'function') {
cb(str);
return cb(null, str);
}
});

parser.onError(function(error) {
if (typeof cb === 'function') {
return cb(error);
}

// No callback to handle the error?
throw error;
});

return str;
Expand Down
35 changes: 32 additions & 3 deletions src/htmlparser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,11 @@ function HTMLParser(html, handler) {
text,
prevTag,
nextTag,
function() {
function(error) {
if (error) {
return handleError(error);
}

prevTag = '';
checkForParseError();
}
Expand All @@ -252,7 +256,11 @@ function HTMLParser(html, handler) {
charsText,
null,
null,
function() {
function(error) {
if (error) {
return handleError(error);
}

html = html.substring(0, replaceDetails.index) + html.substring(replaceDetails.index + replaceDetails[0].length);
parseEndTag('</' + stackedTag + '>', stackedTag);
checkForParseError();
Expand All @@ -267,7 +275,7 @@ function HTMLParser(html, handler) {

function checkForParseError() {
if (html === last) {
throw new Error('Parse Error: ' + html);
return handleError(new Error('Parse Error: ' + html));
}

return parse();
Expand All @@ -286,6 +294,13 @@ function HTMLParser(html, handler) {
}
}

function handleError(error) {
$this.error = error;
if ($this.onErrorCallback) {
$this.onErrorCallback(error);
}
}

function parseStartTag(input) {
var start = input.match(startTagOpen);
if (start) {
Expand Down Expand Up @@ -452,6 +467,20 @@ HTMLParser.prototype.onComplete = function(cb) {
}
};

/**
* Called when the HTMLParser encounters an error.
*
* @param {Function} cb - Callback function.
*/
HTMLParser.prototype.onError = function(cb) {
if (this.error) {
cb(this.error);
}
else {
this.onErrorCallback = cb;
}
};

exports.HTMLParser = HTMLParser;
exports.HTMLtoXML = function(html) {
var results = '';
Expand Down
61 changes: 52 additions & 9 deletions tests/minifier.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3411,7 +3411,8 @@ QUnit.test('style minification with callback', function(assert) {
}, 0);
}
},
function(result) {
function(error, result) {
assert.notOk(error);
assert.equal(result, output);
done();
}
Expand All @@ -3431,7 +3432,8 @@ QUnit.test('script minification with callback', function(assert) {
}, 0);
}
},
function(result) {
function(error, result) {
assert.notOk(error);
assert.equal(result, output);
done();
}
Expand All @@ -3452,7 +3454,8 @@ QUnit.test('style async minification with script sync minification', function(as
},
minifyJS: true
},
function(result) {
function(error, result) {
assert.notOk(error);
assert.equal(result, output);
done();
}
Expand All @@ -3473,7 +3476,8 @@ QUnit.test('style sync minification with script async minification', function(as
}, 0);
}
},
function(result) {
function(error, result) {
assert.notOk(error);
assert.equal(result, output);
done();
}
Expand All @@ -3498,7 +3502,8 @@ QUnit.test('style async minification with script async minification', function(a
}, 0);
}
},
function(result) {
function(error, result) {
assert.notOk(error);
assert.equal(result, output);
done();
}
Expand Down Expand Up @@ -3526,7 +3531,8 @@ QUnit.test('async minification along side sync minification', function(assert) {
}, 0);
}
},
function(result) {
function(error, result) {
assert.notOk(error);
assert.equal(result, asyncOutput);
done();
}
Expand Down Expand Up @@ -3557,7 +3563,8 @@ QUnit.test('multiple async minifications', function(assert) {
}, 0);
}
},
function(result) {
function(error, result) {
assert.notOk(error);
assert.equal(result, output);
done();
}
Expand All @@ -3577,7 +3584,8 @@ QUnit.test('multiple async minifications', function(assert) {
}, 0);
}
},
function(result) {
function(error, result) {
assert.notOk(error);
assert.equal(result, output);
done();
}
Expand All @@ -3591,9 +3599,44 @@ QUnit.test('sync minify with callback', function(assert) {
assert.equal(minify(
input,
{},
function(result) {
function(error, result) {
assert.notOk(error);
assert.equal(result, output);
done();
}
), output);
});


QUnit.test('minify error with callback', function(assert) {
var input = '<style>div#foo { background-color: red; }</style><invalid html';
var done = assert.async();

minify(
input,
{},
function(error) {
assert.ok(error);
done();
}
);
});

QUnit.test('error in callback', function(assert) {
var input = '<style>div#foo { background-color: red; }</style>';
var error = new Error();
var done = assert.async();

minify(
input,
{
minifyCSS: function() {
throw error;
}
},
function(err) {
assert.strictEqual(err, error);
done();
}
);
});