Callback Support #907

RebeccaStevens · 2018-04-11T07:32:32Z

Adds callback support to minify, minifyCSS and minifyJS functions.
Does not break any existing functionality.

Example Use:

var minify = require('html-minifier').minify;
var html = '<html><head><style>body { margin: 0; display: flex; }</style></head></html>';

var config = {
  minifyCSS: function(css, cb) {
    doSomethingAsync(css, function(updatedCss) {
      cb(updatedCss);
    });
  },
  minifyJS: function(js, inline, cb) {
    doSomethingElseAsync(js, function(updatedJs) {
      cb(updatedJs);
    });
  }
};

minify(html, config, function(error, result) {
  if (error) {
    // Handle error.
  } else {
    // Do something with the results.
  }
});

Fixes #906

…cks.

…t when a callback is given, if it finished synchronously.

…backs.

… be postponed.

… has finished parsing the code before continuing.

…completes. minifyJS and minifyCSS function are now passed a callback function.

…inue the parse loop once called.

…ions.

RebeccaStevens · 2018-04-11T07:33:53Z

I plan on adding documentation updates soon.

alexlamsl · 2018-04-11T07:37:11Z

As stated in #906 (comment), you need to preserve the signature of minify(), i.e. it needs to return a string that is the minified input.

Any changes to that interface would be a breaking change and cannot be accepted.

RebeccaStevens · 2018-04-11T07:41:01Z

The signature of minify() is preserve for synchronous calls. It only differs for asynchronous calls.

alexlamsl · 2018-04-11T07:43:05Z

I see.

Before I can perform a code review, please exclude any auto-generated files from the changeset, i.e. dist/*.js

RebeccaStevens · 2018-04-11T07:47:10Z

Changes to dist files have now been undone.

RebeccaStevens · 2018-04-11T07:54:40Z

Just did a rebase.

alexlamsl · 2018-04-11T08:14:39Z

Thanks - looking at the changes, one question jumps out - is HTMLParser still functional synchronously?

Apologies for not mentioning this earlier - there are users would rely on that to perform pre/post-processing, outside the scope of minification.

One way I can think of is similar to ignoreCustomFragments, i.e. assign temporary (unique) strings as placeholders for asynchronous minify{CS,J}S. That way the changes would also be confined within src/htmlminifier.js

alexlamsl · 2018-04-11T08:18:47Z

src/htmlminifier.js

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


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

RebeccaStevens · 2018-04-11T08:34:26Z

HTMLParser is synchronous when minifyCSS and minifyJS are synchronous.

Does HTMLParser also need to be made synchronous for when either minifyCSS or minifyJS are asynchronous?

alexlamsl · 2018-04-11T10:22:06Z

HTMLParser is synchronous when minifyCSS and minifyJS are synchronous

I seem to have trouble validating that - the following test file is used:

// test.js
var HTMLParser = require('./src/htmlparser').HTMLParser;
var input = require('fs').readFileSync('./index.html', 'utf8');

function log(prefix) {
  return function() {
    var args = [].slice.call(arguments);
    args.unshift(prefix);
    console.log.apply(console, args);
  };
}

new HTMLParser(input, {
  start: log('start'),
  end: log('end'),
  chars: log('chars'), 
  comment: log('comment')
});

On current HEAD of gh-pages:

$ cat test.js | node
chars
  html
start html [ { name: 'lang',
    value: 'en',
    customAssign: '=',
    customOpen: '',
    customClose: '',
    quote: '"' } ] false
chars
   html head
start head [] false
... snip many lines in between ...
end body [] false
chars
 /body /html
end html [ { name: 'lang',
    value: 'en',
    customAssign: '=',
    customOpen: '',
    customClose: '',
    quote: '"' } ] false
chars
 /html

... whereas with this PR:

$ cat test.js | node
chars
  html function (error) {
            if (error) {
              return handleError(error);
            }

            prevTag = '';
            checkForParseError();
          }

RebeccaStevens · 2018-04-11T10:35:34Z

I'll look into that tomorrow and make the necessary change to get it working.

Rebecca Stevens added 13 commits April 11, 2018 16:42

Added tests for async style and script minification using callbacks.

8a675e4

Added more tests for async style and script minification using callba…

7f1dc9c

…cks.

Added a test to ensure that the minify function still results a resul…

8a3105a

…t when a callback is given, if it finished synchronously.

Added callback parameter to minify.

a68ee19

Added callback parameter to minifyCSS and minifyJS.

892ca4b

Added an onComplete function to the the HTMLParser class.

77f3786

Switch out the iterative loop for a recursive loop that supports call…

127669d

…backs.

Wrapped the parse error check up into a function so its execution can…

16c9ab7

… be postponed.

Refactored HTMLParser's parse loop.

9c447fd

Code at the end of the minify function now waits until the HTMLParser…

bac6a0e

… has finished parsing the code before continuing.

Added callback to chars function which will always be called once it …

b2626ce

…completes. minifyJS and minifyCSS function are now passed a callback function.

HTMLParser now supplies the chars's callback function which will cont…

143a8fe

…inue the parse loop once called.

Moved parse() call up so that it isn't hidden within function definit…

e487a50

…ions.

Rebecca Stevens added 2 commits April 11, 2018 19:52

Added error handling.

b51fe21

Fixed comment.

76007a1

alexlamsl reviewed Apr 11, 2018

View reviewed changes

Switch out undefined compare for typeof undefined compare.

65d142d

RebeccaStevens mentioned this pull request Apr 12, 2018

Callback Support (alternative implementation) #908

Open

alexlamsl force-pushed the gh-pages branch from 826aa39 to 43eb7dc Compare April 16, 2018 18:04

abdonrd mentioned this pull request Sep 11, 2020

Allow async functions for minifyCSS and minifyJS terser/html-minifier-terser#49

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Callback Support #907

Callback Support #907

RebeccaStevens commented Apr 11, 2018

RebeccaStevens commented Apr 11, 2018

alexlamsl commented Apr 11, 2018

RebeccaStevens commented Apr 11, 2018 •

edited

Loading

alexlamsl commented Apr 11, 2018

RebeccaStevens commented Apr 11, 2018

RebeccaStevens commented Apr 11, 2018

alexlamsl commented Apr 11, 2018

alexlamsl Apr 11, 2018

RebeccaStevens Apr 11, 2018

RebeccaStevens commented Apr 11, 2018

alexlamsl commented Apr 11, 2018

RebeccaStevens commented Apr 11, 2018 •

edited

Loading

Callback Support #907

Are you sure you want to change the base?

Callback Support #907

Conversation

RebeccaStevens commented Apr 11, 2018

Example Use:

RebeccaStevens commented Apr 11, 2018

alexlamsl commented Apr 11, 2018

RebeccaStevens commented Apr 11, 2018 • edited Loading

alexlamsl commented Apr 11, 2018

RebeccaStevens commented Apr 11, 2018

RebeccaStevens commented Apr 11, 2018

alexlamsl commented Apr 11, 2018

alexlamsl Apr 11, 2018

Choose a reason for hiding this comment

RebeccaStevens Apr 11, 2018

Choose a reason for hiding this comment

RebeccaStevens commented Apr 11, 2018

alexlamsl commented Apr 11, 2018

RebeccaStevens commented Apr 11, 2018 • edited Loading

RebeccaStevens commented Apr 11, 2018 •

edited

Loading

RebeccaStevens commented Apr 11, 2018 •

edited

Loading