Conversion to Typescript #712
Replies: 10 comments 5 replies
-
|
Yes I think it's about time this module offered typescript. As long as NPM
install in a non-typescript project is completely brainless, zero change in
that pattern, we would be all for it.
THOMAS BOUTELL | CHIEF EXECUTIVE OFFICER
APOSTROPHECMS | apostrophecms.com | he/him/his
…On Sat, Sep 27, 2025, 4:09 PM RampantDespair ***@***.***> wrote:
Hey there,
I wanted to ask whether you’d be open to a pull request that converts the
codebase to TypeScript, while retaining all current functionality. This
would provide built-in type safety, better editor/IDE support, and official
type definitions without changing the existing API.
Would this be something the maintainers are interested in?
Let me know!
—
Reply to this email directly, view it on GitHub
<#712>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAH27L3KZXZC6ZAIAP6RAL3U3VI7AVCNFSM6AAAAACHVUBS2SVHI2DSMVQWIX3LMV43ERDJONRXK43TNFXW4OZYHE2TENJUGM>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Beta Was this translation helpful? Give feedback.
-
|
Alright, I'm on it, I also had ideas for new features which I'll propose once I'm done with the initial migration. |
Beta Was this translation helpful? Give feedback.
-
|
Hello, I should mention that I have a tendency to get carried away when upgrading a project, and it’s possible that I’ve strayed a bit from the original design philosophy. One of the bigger changes I made was switching from htmlparser2 to parse5, which essentially breaks the performance-first approach in exchange for robustness. Personally, I think robustness is more valuable when it comes to the security-related aspects of a project, but I understand that’s a significant shift. I also added attribute value validation, and I’m considering adding zod v4 as a dependency to reduce the overhead of remaking a validator stack. In any case, I should be done by the end of this week hopefully and the changes will be pushed to my fork. |
Beta Was this translation helpful? Give feedback.
-
|
I see. Since sanitize-html currently allows options to be passed on to
htmlparser2, this would be a backwards compatibility break, therefore a new
major version. That doesn't mean it's not possible.
I'd like to know in what ways htmlparser2 is not considered robust - are we
talking about ways to straight-up bypass sanitization or something else?
Can the performance impact be quantified?
…On Wed, Oct 1, 2025 at 5:22 PM RampantDespair ***@***.***> wrote:
Hello,
I should mention that I have a tendency to get carried away when upgrading
a project, and it’s possible that I’ve strayed a bit from the original
design philosophy.
One of the bigger changes I made was switching from htmlparser2
<https://www.npmjs.com/package/htmlparser2> to parse5
<https://www.npmjs.com/package/parse5>, which essentially breaks the
performance-first approach in exchange for robustness. Personally, I think
robustness is more valuable when it comes to the security-related aspects
of a project, but I understand that’s a significant shift.
I also added attribute value validation, and I’m considering adding zod v4
<https://zod.dev/> as a dependency to reduce the overhead of remaking a
validator stack.
In any case, I should be done by the end of this week hopefully and the
changes will be pushed to my fork.
I'll notify you by then, and I’m happy to collaborate and see how these
changes fit with the project’s direction.
—
Reply to this email directly, view it on GitHub
<#712 (reply in thread)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAH27OVROTBPQJ367RPEKD3VRA3DAVCNFSM6AAAAACHVUBS2SVHI2DSMVQWIX3LMV43URDJONRXK43TNFXW4Q3PNVWWK3TUHMYTINJWG43DCNQ>
.
You are receiving this because you commented.Message ID:
***@***.***
.com>
--
THOMAS BOUTELL | CHIEF EXECUTIVE OFFICER
APOSTROPHECMS | apostrophecms.com | he/him/his
|
Beta Was this translation helpful? Give feedback.
-
|
Let me clarify why I suggested switching from htmlparser2 to parse5:
One of the reasons that got me to explore alternatives was this: Lines 668 to 672 in 86efc06 parse5 removes the need to manually parse html and I thought this was a good thing, considering the library should focus on sanitization more than the parsing itself.
In short: parse5 makes the sanitizer more spec-accurate and resistant to odd inputs; the install flow will remain unchanged; and I’ll provide benchmark data so you can make an informed decision on whether the trade-off is acceptable. |
Beta Was this translation helpful? Give feedback.
-
|
You make good points here. As long as it provides tolerance in the same way web browsers do while producing a more compliant result, I think parse5 is a good candidate to replace htmlparser2 in a way that could address some longstanding edge cases. If we're going to bump the major version though, I think we should look at moving the CSS parsing support to an optional secondary module. I say that because it introduces a dependency on postcss, which leads to many complaints about the difficulty of using the module in the browser. While we don't use it in the browser, there are use cases for that, and it would be nice if the default install of sanitize-html was more easily compatible with installation on the browser side, in exchange for making CSS parsing an optional external module that does depend on postcss. I'm not saying you have to do that part yourself to get a PR accepted, but it would be on the roadmap to actually releasing any incompatible new version. I think we'll want to create a new |
Beta Was this translation helpful? Give feedback.
-
|
Hi @boutell, I wanted to let you know that I’ve pushed a working iteration (in my fork). I’ve put it through quite a bit of testing against edge cases and invalid markup, and so far the results look promising. However, I want to flag that this current state is still more of a proof-of-concept than a production-ready pull request. It’s not yet ready to merge because:
That said, I’m actively working on a “defaults suite” concept: a collection of helpers and default configurations that cover all MDN’s HTML elements and allowed attributes, with sane defaults out of the box. The hope is that such tooling can make adoption simpler, and help ensure consistency between the TypeScript version and what users expect today. And to be clear, the "defaults suite" and the default config are 2 separate things. However obviously the default config will rely on the suite. As an example: referrerpolicy: {
mode: "simple",
value: [
"no-referrer",
"no-referrer-when-downgrade",
"origin",
"origin-when-cross-origin",
"same-origin",
"strict-origin",
"strict-origin-when-cross-origin",
"unsafe-url",
],
},In this case, the attribute Happy to get your thoughts or feedback on whether this path looks promising. |
Beta Was this translation helpful? Give feedback.
-
|
Interesting. It sounds like you're aiming to go beyond just adding type
safety for what developers do (configure options, etc) and also do new
forms of correctness checking on actual inputs, at runtime (such as your
referrerpolicy example). Am I reading that right:?
I would recommend focusing on type safety for calls to the function — are
these arguments legal, yes or no — and not trying to tackle new kinds of
runtime validation in the same PR.
…On Thu, Oct 9, 2025 at 4:42 PM RampantDespair ***@***.***> wrote:
Hi @boutell <https://github.com/boutell>,
I wanted to let you know that I’ve pushed a working iteration (in my
fork). I’ve put it through quite a bit of testing against edge cases and
invalid markup, and so far the results look promising.
However, I want to flag that *this current state is still more of a
proof-of-concept* than a production-ready pull request. It’s not yet
ready to merge because:
- It lacks full coverage, or I should say sufficient coverage (because
realistically full coverage for a library like this is pretty much
impossible considering all the different intricacies in html elements...)
- Haven't gotten the time to make the promised benchmark
That said, I’m actively working on a “defaults suite” concept: a
collection of helpers and default configurations that cover all MDN’s HTML
elements and allowed attributes, with sane defaults out of the box. The
hope is that such tooling can make adoption simpler, and help ensure
consistency between the TypeScript version and what users expect today.
And to be clear, the "defaults suite" and the default config are 2
separate things. However obviously the default config will rely on the
suite.
As an example:
referrerpolicy: {
mode: "simple",
value: [
"no-referrer",
"no-referrer-when-downgrade",
"origin",
"origin-when-cross-origin",
"same-origin",
"strict-origin",
"strict-origin-when-cross-origin",
"unsafe-url",
],
},
In this case, the attribute referrerpolicy expects a one of the values
indicated as directed by MDN (
https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/a#referrerpolicy
).
Happy to get your thoughts or feedback on whether this path looks
promising.
—
Reply to this email directly, view it on GitHub
<#712 (reply in thread)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAH27KWUEWKAVHCKFTUIUT3W3CFBAVCNFSM6AAAAACHVUBS2SVHI2DSMVQWIX3LMV43URDJONRXK43TNFXW4Q3PNVWWK3TUHMYTINRUGEYDAOA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***
.com>
--
THOMAS BOUTELL | CHIEF EXECUTIVE OFFICER
APOSTROPHECMS | apostrophecms.com | he/him/his
|
Beta Was this translation helpful? Give feedback.
-
|
Which is to say: let typescript be typescript and do its compile-time job
well, but don't try to add more runtime checking (which typescript doesn't
do).
…On Fri, Oct 10, 2025 at 10:26 AM Tom Boutell ***@***.***> wrote:
Interesting. It sounds like you're aiming to go beyond just adding type
safety for what developers do (configure options, etc) and also do new
forms of correctness checking on actual inputs, at runtime (such as your
referrerpolicy example). Am I reading that right:?
I would recommend focusing on type safety for calls to the function — are
these arguments legal, yes or no — and not trying to tackle new kinds of
runtime validation in the same PR.
On Thu, Oct 9, 2025 at 4:42 PM RampantDespair ***@***.***>
wrote:
> Hi @boutell <https://github.com/boutell>,
>
> I wanted to let you know that I’ve pushed a working iteration (in my
> fork). I’ve put it through quite a bit of testing against edge cases and
> invalid markup, and so far the results look promising.
>
> However, I want to flag that *this current state is still more of a
> proof-of-concept* than a production-ready pull request. It’s not yet
> ready to merge because:
>
> - It lacks full coverage, or I should say sufficient coverage
> (because realistically full coverage for a library like this is pretty much
> impossible considering all the different intricacies in html elements...)
> - Haven't gotten the time to make the promised benchmark
>
> That said, I’m actively working on a “defaults suite” concept: a
> collection of helpers and default configurations that cover all MDN’s HTML
> elements and allowed attributes, with sane defaults out of the box. The
> hope is that such tooling can make adoption simpler, and help ensure
> consistency between the TypeScript version and what users expect today.
>
> And to be clear, the "defaults suite" and the default config are 2
> separate things. However obviously the default config will rely on the
> suite.
>
> As an example:
>
> referrerpolicy: {
> mode: "simple",
> value: [
> "no-referrer",
> "no-referrer-when-downgrade",
> "origin",
> "origin-when-cross-origin",
> "same-origin",
> "strict-origin",
> "strict-origin-when-cross-origin",
> "unsafe-url",
> ],
> },
>
> In this case, the attribute referrerpolicy expects a one of the values
> indicated as directed by MDN (
> https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/a#referrerpolicy
> ).
>
> Happy to get your thoughts or feedback on whether this path looks
> promising.
>
> —
> Reply to this email directly, view it on GitHub
> <#712 (reply in thread)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAAH27KWUEWKAVHCKFTUIUT3W3CFBAVCNFSM6AAAAACHVUBS2SVHI2DSMVQWIX3LMV43URDJONRXK43TNFXW4Q3PNVWWK3TUHMYTINRUGEYDAOA>
> .
> You are receiving this because you were mentioned.Message ID:
> <apostrophecms/sanitize-html/repo-discussions/712/comments/14641008@
> github.com>
>
--
THOMAS BOUTELL | CHIEF EXECUTIVE OFFICER
APOSTROPHECMS | apostrophecms.com | he/him/his
--
THOMAS BOUTELL | CHIEF EXECUTIVE OFFICER
APOSTROPHECMS | apostrophecms.com | he/him/his
|
Beta Was this translation helpful? Give feedback.
-
In part yes, basically the idea is to give the developers a way to sanitize everything down stream, tag -> tagAttribute -> tagAttributeValue. export const defaultTags: DefaultTags = {
// https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/a
a: {
attributes: {
download: {}, // TODO
href: defaultOtherAttributes.url,
hreflang: {}, // TODO
ping: defaultOtherAttributes.urls,
referrerpolicy: defaultOtherAttributes.referrerpolicy,
rel: defaultSharedAttributes.rel,
target: defaultOtherAttributes.target,
type: {}, // TODO
},
},
[...]
Those are already done. |
Beta Was this translation helpful? Give feedback.
-
|
Sorry for the radio silence, I'm busy with something else at this time. |
Beta Was this translation helpful? Give feedback.
-
|
Regrettably I haven't had a chance yet.
…On Thu, Oct 23, 2025 at 2:43 PM RampantDespair ***@***.***> wrote:
Sorry for the radio silence, I'm busy with something else at this time.
Did you get a chance to look at the logic? If so what do you think?
—
Reply to this email directly, view it on GitHub
<#712 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAH27MCPHJIBQ626R6KOZD3ZEOTJAVCNFSM6AAAAACHVUBS2SVHI2DSMVQWIX3LMV43URDJONRXK43TNFXW4Q3PNVWWK3TUHMYTINZWGU2DSMQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***
.com>
--
THOMAS BOUTELL | CHIEF EXECUTIVE OFFICER
APOSTROPHECMS | apostrophecms.com | he/him/his
|
Beta Was this translation helpful? Give feedback.
-
|
@boutell |
Beta Was this translation helpful? Give feedback.
-
|
Sorry about this - we're in serious end of year crunch time over here.
…On Sun, Nov 16, 2025 at 11:23 AM RampantDespair ***@***.***> wrote:
@boutell <https://github.com/boutell>
Any news?
—
Reply to this email directly, view it on GitHub
<#712 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAH27IO6C4ND3XVUQ5BDW335CQJHAVCNFSM6AAAAACHVUBS2SVHI2DSMVQWIX3LMV43URDJONRXK43TNFXW4Q3PNVWWK3TUHMYTIOJYGIZDKMI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***
.com>
--
THOMAS BOUTELL | CHIEF EXECUTIVE OFFICER
APOSTROPHECMS | apostrophecms.com | he/him/his
|
Beta Was this translation helpful? Give feedback.
-
|
@boutell |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
Hey there,
I wanted to ask whether you’d be open to a pull request that converts the codebase to TypeScript, while retaining all current functionality. This would provide built-in type safety, better editor/IDE support, and official type definitions without changing the existing API.
Would this be something the maintainers are interested in?
Let me know!
Beta Was this translation helpful? Give feedback.
All reactions