apostrophecms / sanitize-html

Clean up user-submitted HTML, preserving whitelisted elements and whitelisted attributes on a per-element basis. Built on htmlparser2 for speed and tolerance
MIT License
3.84k stars 353 forks source link

Update sanitize-html from 2.7.3 to 2.8.0 fails under webpack 4: Module parse failed: Unexpected token #592

Closed alexander-schranz closed 1 year ago

alexander-schranz commented 1 year ago

Hi, we are using ckeditor which is using sanitze-html inside it. And it seems like with the latest release it does not longer support webpack 4?

To Reproduce

Step by step instructions to reproduce the behavior:

  1. composer create-project sulu/skeleton # (requires composer)
  2. cd skeleton/assets/install
  3. npm install
  4. npm run build

Errors now with:

ERROR in ./node_modules/htmlparser2/lib/esm/index.js 59:9 Module parse failed: Unexpected token (59:9) You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders | return getFeed(parseDOM(feed, options)); | }

export * as DomUtils from "domutils"; | // Old name for DomHandler | export { DomHandler as DefaultHandler };

Expected behavior

Build should work as expected.

Describe the bug

The update from htmlparser2 from 6 to 8 seems to crash this. Seems like htmlparser2 8 version does not support webpack 4 terser parser?

Details

Version of Node.js: 14

Server Operating System: MacOS

Additional context:

Webpack 4 Terser Parser.

Screenshots

Bildschirmfoto 2022-12-13 um 13 52 11

alexander-schranz commented 1 year ago

I found a workaround which I wanted to share here if someone else stuble over it. The workaround is to run the whole htmlparser2 through babel currently. In my case I also needed to extend the webpack.config.js to add the htmlparser2 be loaded over the babel-loader:

diff --git a/babel.config.json b/babel.config.json
index f22d5f2bb4..757b4dacc6 100644
--- a/babel.config.json
+++ b/babel.config.json
@@ -8,7 +8,8 @@
         "@babel/plugin-transform-flow-strip-types",
         "@babel/plugin-proposal-nullish-coalescing-operator",
         "@babel/plugin-proposal-class-properties",
-        "@babel/plugin-proposal-optional-chaining"
+        "@babel/plugin-proposal-optional-chaining",
+        "@babel/plugin-proposal-export-namespace-from"
     ],
     "assumptions": {
         "setPublicClassFields": true
diff --git a/package.json b/package.json
index 5830486962..63270c671d 100644
--- a/package.json
+++ b/package.json
@@ -35,6 +35,7 @@
         "@babel/core": "^7.5.5",
         "@babel/plugin-proposal-class-properties": "^7.16.5",
         "@babel/plugin-proposal-decorators": "^7.4.4",
+        "@babel/plugin-proposal-export-namespace-from": "^7.18.9",
         "@babel/plugin-proposal-nullish-coalescing-operator": "^7.14.2",
         "@babel/plugin-proposal-optional-chaining": "^7.18.9",
         "@babel/plugin-transform-flow-strip-types": "^7.4.4",
diff --git a/webpack.config.js b/webpack.config.js
index 563a3d48c8..caa8711162 100644
--- a/webpack.config.js
+++ b/webpack.config.js
@@ -85,7 +85,7 @@ module.exports = (env, argv) => { // eslint-disable-line no-undef
                 {
                     test: /\.js$/,
                     // eslint-disable-next-line max-len
-                    exclude: /node_modules[/\\](?!(sulu-(.*)-bundle|@ckeditor|ckeditor5|array-move|lodash-es|@react-leaflet|react-leaflet)[/\\])/,
+                    exclude: /node_modules[/\\](?!(sulu-(.*)-bundle|@ckeditor|ckeditor5|array-move|htmlparser2|lodash-es|@react-leaflet|react-leaflet)[/\\])/,
                     use: {
                         loader: 'babel-loader',
                         options: {

Update: Found other related issues here:

boutell commented 1 year ago

Thanks for providing a solution for those who encounter this. I agree it's not a bug (it's using valid JS syntax) but helpful to have a workaround for those who use sanitize-html on the front end in an older build stack.

On Tue, Dec 13, 2022 at 8:40 AM Alexander Schranz @.***> wrote:

Closed #592 https://github.com/apostrophecms/sanitize-html/issues/592 as completed.

— Reply to this email directly, view it on GitHub https://github.com/apostrophecms/sanitize-html/issues/592#event-8025601529, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27KNR54BVSMAVG6CSW3WNB4D5ANCNFSM6AAAAAAS5FSIPU . You are receiving this because you are subscribed to this thread.Message ID: @.*** com>

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

reisnobre commented 1 year ago

I'm still having this issue. I tried everything but no such luck. Any ideas?

Details

Version of Node.js: 12.22

Server Operating System: MacOS

Additional context:

Using laravel-mix 5.