Skip to content

Commit

Permalink
Switch XML minifier from pretty-data to minify-xml
Browse files Browse the repository at this point in the history
All in all pretty-data is a pretty bad (phun intended) XML minifier. It
does for instance not minify tag contents, nor it optimizes anything
else than removing comments and whitespaces between tags. Tags like:

<Tag     attributeA   =   "..."     attributeB = "..."     />

Will not get reduced in size. Switched to minify-xml (a package which
I specifically created for this patch, to do a better job at minfying)
on the other hands removes comments and whitespaces between tags, as
well as collapses whitespaces in tags and removes unused namespaces from
views. These are all pretty common use-cases for UI5 xml views, thus I
clearly think it's the better option compared to pretty-data.

Also this change removes the strange assumption that pretty-data (or now
minify-xml) would modify any tag content (like pre tags), which both
libraries clearly don't do. Fixed typo in test (autpSplitter).
  • Loading branch information
kristian committed Oct 23, 2020
1 parent 4e54820 commit ace614f
Show file tree
Hide file tree
Showing 9 changed files with 117 additions and 142 deletions.
16 changes: 8 additions & 8 deletions lib/lbt/bundle/AutoSplitter.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
"use strict";

const terser = require("terser");
const {pd} = require("pretty-data");
const minifyXml = require("minify-xml");

const ModuleName = require("../utils/ModuleName");
const {SectionType} = require("./BundleDefinition");
const escapePropertiesFile = require("../utils/escapePropertiesFile");
const log = require("@ui5/logger").getLogger("lbt:bundle:AutoSplitter");

const copyrightCommentsPattern = /copyright|\(c\)(?:[0-9]+|\s+[0-9A-za-z])|released under|license|\u00a9/i;
const xmlHtmlPrePattern = /<(?:\w+:)?pre>/;
const xmlHtmlPrePattern = /<(?:\w+:)?pre\b/;

/**
*
Expand Down Expand Up @@ -235,12 +235,12 @@ class AutoSplitter {
// needs to be activated when it gets activated in JSMergedModuleBuilderExt
let fileContent = await resource.buffer();
if ( this.optimize ) {
// For XML we use the pretty data
// Do not minify if XML(View) contains an <*:pre> tag because whitespace of
// HTML <pre> should be preserved (should only happen rarely)
if (!xmlHtmlPrePattern.test(fileContent)) {
fileContent = pd.xmlmin(fileContent, false);
}
// use minify-xml for XML, do not minify whitespaces between tags if the XML(View) contains an
// <*:pre> tag, because whitespace of HTML <pre> should be preserved (this should rarely happen)
const noPreTag = !xmlHtmlPrePattern.test(fileContent);
fileContent = minifyXml.minify(fileContent.toString(), {
removeWhitespaceBetweenTags: noPreTag
});
}
return fileContent.length;
}
Expand Down
16 changes: 8 additions & 8 deletions lib/lbt/bundle/Builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"use strict";

const terser = require("terser");
const {pd} = require("pretty-data");
const minifyXml = require("minify-xml");
const esprima = require("esprima");
const escodegen = require("escodegen");
const {Syntax} = esprima;
Expand All @@ -21,7 +21,7 @@ const BundleWriter = require("./BundleWriter");
const log = require("@ui5/logger").getLogger("lbt:bundle:Builder");

const copyrightCommentsPattern = /copyright|\(c\)(?:[0-9]+|\s+[0-9A-za-z])|released under|license|\u00a9|^@ui5-bundle-raw-include |^@ui5-bundle /i;
const xmlHtmlPrePattern = /<(?:\w+:)?pre>/;
const xmlHtmlPrePattern = /<(?:\w+:)?pre\b/;

const strReplacements = {
"\r": "\\r",
Expand Down Expand Up @@ -426,12 +426,12 @@ class BundleBuilder {
} else if ( /\.xml$/.test(module) ) {
let fileContent = await resource.buffer();
if ( this.optimize ) {
// For XML we use the pretty data
// Do not minify if XML(View) contains an <*:pre> tag,
// because whitespace of HTML <pre> should be preserved (should only happen rarely)
if (!xmlHtmlPrePattern.test(fileContent)) {
fileContent = pd.xmlmin(fileContent.toString(), false);
}
// use minify-xml for XML, do not minify whitespaces between tags if the XML(View) contains an
// <*:pre> tag, because whitespace of HTML <pre> should be preserved (this should rarely happen)
const noPreTag = !xmlHtmlPrePattern.test(fileContent);
fileContent = minifyXml.minify(fileContent.toString(), {
removeWhitespaceBetweenTags: noPreTag
});
}
outW.write( makeStringLiteral( fileContent ) );
} else if ( /\.properties$/.test(module) ) {
Expand Down
Loading

0 comments on commit ace614f

Please sign in to comment.