search

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.68k stars 349 forks source link

htmlparser2 major version update causes issues with Typescript #601

Open perezal opened 1 year ago

perezal commented 1 year ago

A minor version update to sanitize-html included a major version update to htmlparser2, causing build issues with Typescript v4.2.4. Temporarily fixed by locking sanitize-html version at 4.2.0, but I wonder if minor version updates should include changes that aren't backwards compatible?

To Reproduce

Step by step instructions to reproduce the behavior:

  1. Use sanitize-html with Typescript v4.2.4
  2. Attempt to build

Expected behavior

It builds successfully.

Describe the bug

The major version upgrade to htmlparser2 causes build issue TS1005 with older minor versions of Typescript (using 4.2.4 vs 4.9.4 latest). Should minor version increments include major version dependency updates?

Details

Version of Node.js: Node v16.19.0

Server Operating System: MacOS X Monterey 12.6.1, also presents the same issue in Docker v20.10.11 using node:16-buster

boutell commented 1 year ago

Typescript support for this module is community maintained, we do not use Typescript in-house and are not really in a position to test and maintain that. So my question is whether the typings file just needs to be updated by interested community members in some way or if there is a larger issue with the latest version of htmlparser2. But to my knowledge there is no reason why compiling a javascript (not typescript) submodule should bother Typescript at all.

arcreative commented 8 months ago

I've had the same issue. Very concerning that a minor version upgrade could cause this, as it was quite hard to track down...

boutell commented 8 months ago

What is the issue, exactly? Where in the sanitize-html API is it exposed? Thanks.

dylanarmstrong commented 4 months ago

@boutell, It's an issue when using sanitize-html with typescript < 4.5. The htmlparser2 types are not supported on typescript versions that low: https://github.com/fb55/htmlparser2/releases/tag/v8.0.0

I'm not really sure there's anything that can be done here aside from locking the sanitize-html version to one that doesn't include this version.

boutell commented 4 months ago

This is unfortunate - we don't use typescript in-house, and there were no apparent backwards compatibility breaks in javascript in htmlparser2. A PR to lock down the required version of typescript would be welcome.

On Wed, Feb 21, 2024 at 9:51 AM Dylan Armstrong @.***> wrote:

@boutell https://github.com/boutell, It's an issue when using sanitize-html with typescript < 4.5. The htmlparser2 types are not supported on typescript versions that low: https://github.com/fb55/htmlparser2/releases/tag/v8.0.0

I'm not really sure there's anything that can be done here aside from locking the sanitize-html version to one that doesn't include this version.

— Reply to this email directly, view it on GitHub https://github.com/apostrophecms/sanitize-html/issues/601#issuecomment-1956824133, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27KA6EKDQ7JDSLDYRFTYUYCYRAVCNFSM6AAAAAATYSOULWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNJWHAZDIMJTGM . You are receiving this because you were mentioned.Message ID: @.***>

--

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

dylanarmstrong commented 4 months ago

I wasn't able to find anyway to do this that actually worked. Tested a couple ideas on both @types and the sanitize-html package. I think specifying in the README that typescript has to be >= 4.5 is only solution. The types for sanitize-html have a dependency on htmlparser2 types for ParserOptions for the parser method. PR for README here: https://github.com/apostrophecms/sanitize-html/pull/655/files