From 2a3b3ef9da88fd45fee27b08b0323c9f3e5f18c0 Mon Sep 17 00:00:00 2001 From: Edimar Cardoso Date: Wed, 27 Jul 2022 18:03:46 -0300 Subject: [PATCH 1/6] Create async method `CssTools.minifyCssAsync` to replace method `CssTools.minifyCss` in future --- packages/minifier-css/minifier-async-tests.js | 51 +++++++++++++++++++ packages/minifier-css/minifier.js | 41 ++++++++++----- packages/minifier-css/package.js | 1 + .../plugin/minify-css.js | 2 +- 4 files changed, 81 insertions(+), 14 deletions(-) create mode 100644 packages/minifier-css/minifier-async-tests.js diff --git a/packages/minifier-css/minifier-async-tests.js b/packages/minifier-css/minifier-async-tests.js new file mode 100644 index 0000000000..232ca247a2 --- /dev/null +++ b/packages/minifier-css/minifier-async-tests.js @@ -0,0 +1,51 @@ +import { CssTools } from './minifier'; +const TEST_CASES = [ + ['a \t\n{ color: red } \n', 'a{color:red}', 'whitespace check'], + [ + 'a \t\n{ color: red; margin: 1; } \n', + 'a{color:red;margin:1}', + 'only last one loses semicolon', + ], + [ + 'a \t\n{ color: red;;; margin: 1;;; } \n', + 'a{color:red;margin:1}', + 'more semicolons than needed', + ], + ['a , p \t\n{ color: red; } \n', 'a,p{color:red}', 'multiple selectors'], + ['body {}', '', 'removing empty rules'], + [ + '*.my-class { color: #fff; }', + '.my-class{color:#fff}', + 'removing universal selector', + ], + [ + 'p > *.my-class { color: #fff; }', + 'p>.my-class{color:#fff}', + 'removing optional whitespace around ">" in selector', + ], + [ + 'p + *.my-class { color: #fff; }', + 'p+.my-class{color:#fff}', + 'removing optional whitespace around "+" in selector', + ], + [ + 'a {\n\ + font:12px \'Helvetica\',"Arial",\'Nautica\';\n\ + background:url("/some/nice/picture.png");\n}', + 'a{font:12px Helvetica,Arial,Nautica;background:url(/some/nice/picture.png)}', + 'removing quotes in font and url (if possible)', + ], + ['/* no comments */ a { color: red; }', 'a{color:red}', 'remove comments'], +]; + +Tinytest.addAsync( + '[Async] minifier-css - simple CSS minification', + (test, onComplete) => { + const promises = TEST_CASES.map(([css, expected, desc]) => + CssTools.minifyCssAsync(css).then((minifiedCss) => { + test.equal(minifiedCss[0], expected, desc); + }) + ); + Promise.all(promises).then(() => onComplete()); + } +); diff --git a/packages/minifier-css/minifier.js b/packages/minifier-css/minifier.js index 174452f1ee..2fa610519a 100644 --- a/packages/minifier-css/minifier.js +++ b/packages/minifier-css/minifier.js @@ -63,25 +63,39 @@ const CssTools = { * * @param {string} cssText CSS string to minify. * @return {String[]} Array containing the minified CSS. + * @deprecated on 2.8 */ minifyCss(cssText) { - const f = new Future; - postcss([ - cssnano({ safe: true }), - ]).process(cssText, { - from: void 0, - }).then(result => { - f.return(result.css); - }).catch(error => { - f.throw(error); - }); - const minifiedCss = f.wait(); - + const f = new Future(); + CssTools.minifyCssAsync(cssText) + .then((res) => f.return(res)) + .catch((error) => f.throw(error)); // Since this function has always returned an array, we'll wrap the // minified css string in an array before returning, even though we're // only ever returning one minified css string in that array (maintaining // backwards compatibility). - return [minifiedCss]; + return f.wait(); + }, + + /** + * Minify the passed in CSS string. + * + * @param {string} cssText CSS string to minify. + * @return {Promise} Array containing the minified CSS. + */ + async minifyCssAsync(cssText) { + return new Promise((resolve, reject) => { + postcss([cssnano({ safe: true })]) + .process(cssText, { + from: void 0, + }) + .then((result) => { + resolve([result.css]); + }) + .catch((error) => { + reject(error); + }); + }); }, /** @@ -187,6 +201,7 @@ if (typeof Profile !== 'undefined') { 'parseCss', 'stringifyCss', 'minifyCss', + 'minifyCssAsync', 'mergeCssAsts', 'rewriteCssUrls', ].forEach(funcName => { diff --git a/packages/minifier-css/package.js b/packages/minifier-css/package.js index 3447c9e4bf..ffdaf5d5d6 100644 --- a/packages/minifier-css/package.js +++ b/packages/minifier-css/package.js @@ -19,6 +19,7 @@ Package.onTest(function (api) { api.use('tinytest'); api.addFiles([ 'minifier-tests.js', + 'minifier-async-tests.js', 'urlrewriting-tests.js' ], 'server'); }); diff --git a/packages/standard-minifier-css/plugin/minify-css.js b/packages/standard-minifier-css/plugin/minify-css.js index 2b8c4d5e44..8ac2b0db75 100644 --- a/packages/standard-minifier-css/plugin/minify-css.js +++ b/packages/standard-minifier-css/plugin/minify-css.js @@ -60,7 +60,7 @@ class CssToolsMinifier { path: 'merged-stylesheets.css' }]; } else { - const minifiedFiles = CssTools.minifyCss(merged.code); + const minifiedFiles = await CssTools.minifyCssAsync(merged.code); result = minifiedFiles.map(minified => ({ data: minified From 470ae492b0ad7570c626cd3950ba03bfe0b2c79d Mon Sep 17 00:00:00 2001 From: Edimar Cardoso Date: Wed, 27 Jul 2022 18:17:28 -0300 Subject: [PATCH 2/6] Remove unnecessary promise --- packages/minifier-css/minifier.js | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/packages/minifier-css/minifier.js b/packages/minifier-css/minifier.js index 2fa610519a..d22aa02705 100644 --- a/packages/minifier-css/minifier.js +++ b/packages/minifier-css/minifier.js @@ -84,18 +84,14 @@ const CssTools = { * @return {Promise} Array containing the minified CSS. */ async minifyCssAsync(cssText) { - return new Promise((resolve, reject) => { - postcss([cssnano({ safe: true })]) - .process(cssText, { - from: void 0, - }) - .then((result) => { - resolve([result.css]); - }) - .catch((error) => { - reject(error); - }); - }); + return postcss([cssnano({ safe: true })]) + .process(cssText, { + from: void 0, + }) + .then((result) => [result.css]) + .catch((error) => { + throw new Error(error); + }); }, /** From 996ab0b831bfff60e325e1895a7c35f1cc0e2f24 Mon Sep 17 00:00:00 2001 From: Edimar Cardoso Date: Fri, 29 Jul 2022 09:25:30 -0300 Subject: [PATCH 3/6] Using `Promise.await` to resolve the async method. Suggested in Code Review --- packages/minifier-css/minifier.js | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/packages/minifier-css/minifier.js b/packages/minifier-css/minifier.js index d22aa02705..77058554e2 100644 --- a/packages/minifier-css/minifier.js +++ b/packages/minifier-css/minifier.js @@ -1,6 +1,5 @@ import path from 'path'; import url from 'url'; -import Future from 'fibers/future'; import postcss from 'postcss'; import cssnano from 'cssnano'; @@ -66,15 +65,7 @@ const CssTools = { * @deprecated on 2.8 */ minifyCss(cssText) { - const f = new Future(); - CssTools.minifyCssAsync(cssText) - .then((res) => f.return(res)) - .catch((error) => f.throw(error)); - // Since this function has always returned an array, we'll wrap the - // minified css string in an array before returning, even though we're - // only ever returning one minified css string in that array (maintaining - // backwards compatibility). - return f.wait(); + return Promise.await(CssTools.minifyCssAsync(cssText)); }, /** From cbffccb0b9d1b14bb6c5ca0ac3f93919401b9449 Mon Sep 17 00:00:00 2001 From: Edimar Cardoso Date: Fri, 29 Jul 2022 09:30:12 -0300 Subject: [PATCH 4/6] Remove incorrect @deprecated annotation. --- packages/minifier-css/minifier.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/minifier-css/minifier.js b/packages/minifier-css/minifier.js index 77058554e2..c1cb595e23 100644 --- a/packages/minifier-css/minifier.js +++ b/packages/minifier-css/minifier.js @@ -62,7 +62,6 @@ const CssTools = { * * @param {string} cssText CSS string to minify. * @return {String[]} Array containing the minified CSS. - * @deprecated on 2.8 */ minifyCss(cssText) { return Promise.await(CssTools.minifyCssAsync(cssText)); From 5ebf8ff3b2f63877ee665e46bfcf16a5db39a30c Mon Sep 17 00:00:00 2001 From: Edimar Cardoso Date: Fri, 29 Jul 2022 10:17:03 -0300 Subject: [PATCH 5/6] Use await and remove unnecessary catch. --- packages/minifier-css/minifier.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/minifier-css/minifier.js b/packages/minifier-css/minifier.js index c1cb595e23..a4c662e9e5 100644 --- a/packages/minifier-css/minifier.js +++ b/packages/minifier-css/minifier.js @@ -74,14 +74,11 @@ const CssTools = { * @return {Promise} Array containing the minified CSS. */ async minifyCssAsync(cssText) { - return postcss([cssnano({ safe: true })]) + return await postcss([cssnano({ safe: true })]) .process(cssText, { from: void 0, }) - .then((result) => [result.css]) - .catch((error) => { - throw new Error(error); - }); + .then((result) => [result.css]); }, /** From e7060c29f4a3fbaf91d0fe291700f1fb46e6e9e5 Mon Sep 17 00:00:00 2001 From: Edimar Cardoso Date: Fri, 29 Jul 2022 10:27:06 -0300 Subject: [PATCH 6/6] Make method test async and return promise to handle rejection. --- packages/minifier-css/minifier-async-tests.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/minifier-css/minifier-async-tests.js b/packages/minifier-css/minifier-async-tests.js index 232ca247a2..755595ba6e 100644 --- a/packages/minifier-css/minifier-async-tests.js +++ b/packages/minifier-css/minifier-async-tests.js @@ -40,12 +40,12 @@ const TEST_CASES = [ Tinytest.addAsync( '[Async] minifier-css - simple CSS minification', - (test, onComplete) => { + async (test) => { const promises = TEST_CASES.map(([css, expected, desc]) => CssTools.minifyCssAsync(css).then((minifiedCss) => { test.equal(minifiedCss[0], expected, desc); }) ); - Promise.all(promises).then(() => onComplete()); + return Promise.all(promises); } );