Skip to content

Conversation

vitaliytv
Copy link

@R4356th
Copy link
Contributor

R4356th commented Oct 13, 2021

Note that landing this PR as-is will break files using ES modules as this would make html-minifier remove type="module" from script tags making browsers think the modules are general JS files.

@dmitrysmagin
Copy link

dmitrysmagin commented Aug 1, 2022

Hi.
I've faced the same problem and reused this patch for minifying type="module" scripts.
As I see, html-minifier doesn't remove this attribute from the script tag if 'removeScriptTypeAttributes: false', so this patch is kind of safe.

Perhaps, it is possible to make an exception for type="module" when removing attrs for <script> tags, thus this patch would be 100% correct.

@dmitrysmagin
Copy link

diff --git a/node_modules/html-minifier/src/htmlminifier.js b/node_modules/html-minifier/src/htmlminifier.js
index d7efa99..78350a3 100644
--- a/node_modules/html-minifier/src/htmlminifier.js
+++ b/node_modules/html-minifier/src/htmlminifier.js
@@ -155,6 +155,7 @@ function isAttributeRedundant(tag, attrName, attrValue, attrs) {
 // https://mathiasbynens.be/demo/javascript-mime-type
 // https://developer.mozilla.org/en/docs/Web/HTML/Element/script#attr-type
 var executableScriptsMimetypes = utils.createMap([
+  'module',
   'text/javascript',
   'text/ecmascript',
   'text/jscript',
@@ -553,7 +554,7 @@ function normalizeAttr(attr, attrs, tag, options) {
   if (options.removeRedundantAttributes &&
     isAttributeRedundant(tag, attrName, attrValue, attrs) ||
     options.removeScriptTypeAttributes && tag === 'script' &&
-    attrName === 'type' && isScriptTypeAttribute(attrValue) ||
+    attrName === 'type' && isScriptTypeAttribute(attrValue) && attrValue !== 'module' ||
     options.removeStyleLinkTypeAttributes && (tag === 'style' || tag === 'link') &&
     attrName === 'type' && isStyleLinkTypeAttribute(attrValue)) {
     return; 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants