Bundled types are not as well typed as DT types

[oBusk]

dompurify@2.3.11> comes with bundled types that are not as complete as the @types/dompurify which causes build errors with a minor release.

Background & Context

The @types/dompurify types has multiple signatures for sanitize() so that it can ensure what type the function returns. The new bundled type makes sanitize() only returns any, which can cause type issues when you're trying to avoid implicit any.

Bug

Updating to 2.3.12 causes errors because variables that was previously inferred to be string, because of the @types/dompurify signatures.

Input

N/a

Given output

N/a

Expected output

N/a

Feature

The bundled types should also specify multiple signatures to help typescript figure out what type the returned value is.

It seems like having multiple signatures with JSDoc is somewhat complicated.

This is related to #712 but is a separate issue and is not fixed by the .12 patch

[Kasea]

For what purpose is this project supplying types anyway? They aren't close to accurate with reality, so instead of having wrong type support, it's better to have no type support and have the users supply their own types (@types/dompurify in this case)

[RPainter8West]

It's better to have the types maintained within the package otherwise when updates happen there can be a delay before the DT types pick up on the changes.

[cure53]

Hey all, we bundle types because another project had problems without, they are auto-generated here:

https://github.com/cure53/DOMPurify/blob/main/package.json#L19

If anyone has an idea how to generate more precise type definitions so we satisfy both the needs of the other projects and the ones that seem to have issues with 2.3.12, I'd be more than happy to update our code.

However, currently it's a bit of a game of whack-a-mole, so unsure how to address this current issue.

[RPainter8West]

Options:

1. Manually create and update the d.ts files (start by copying from DT versions)
2. Add JsDoc annotations to give TS hints about the types: https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html
3. Rename your JS files to TS and start adding types to your function properties and return values: https://www.typescriptlang.org/docs/handbook/declaration-files/by-example.html

[cure53]

Add JsDoc annotations to give TS hints about the types: https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html

Thanks, this one sounds most practical. So, to address the issue, we would essentially add the following to sanitize() annotations, correct? cc @oBusk

/**
 * [...]
 *
 * @returns { TrustedHTML | string | DocumentFragment | HTMLElement}
 **/

[oBusk]

This will still break most code, arguably more than any.

// Previously inferred as string because we're not passing any cfg
const sanitized = dompurify.sanitize('my input');
// "TrustedHTML has no child 'split'"
sanitized.split(' ');

Of course the workaround is to specify that it will be string manually, but this still means breakage on a patch version.

[swensorm]

Also ran into this issue this morning as well as the Config type was now generically an object and not as well defined as the @types/dompurify definitions. It can be addressed with JsDoc annotations, however you might experience more "whack-a-mole" as you said, as you won't have compile time checking to make sure nothing is missed or inaccurate.

From experience, converting to TS is best though it's not without initial investment. You could cut down the time by only defining types initially for public APIs and use any for everything private.

[cure53]

Then I am frankly clueless how to fix this issue without stepping on anyone's foot. I won't take any further action here unless getting some input on a viable path forward.

[RPainter8West]

@swensorm, this was what I was suggesting as option 3. It's what I would do to. Just declare proper types for function arguments and return values at the start. But I think you can accomplish the same thing with the jsdocs approach. I haven't tried it myself.

[RPainter8West]

@cure53 If you don't feel like you want to take this on now, then the easy option is #1. Just plonk in the file currently on Definitely Typed. If your API surface changes, you'll just need to make those minor adjustments in the d.ts file.

[cure53]

Hm, I think it makes more sense to remove the auto-generated files again and simply direct folks to DefinitelyTyped right away.

This appears to cause too much hassle overall and cannot easily be automated as far as I tell for now.

[cure53]

Closing this for now, latest release (2.4.0 as this might break for people who worked around the types issue) no longer uses bundled types.

[luxaritas]

If the issue is not being able to use the JSDoc approach, I think the key thing is that you need to provide function overloads: https://austingil.com/typescript-function-overloads-with-jsdoc/

As a note WRT verifying types if you really want to use jsdoc and not typescript, typescript does let you do incremental type checking with jsdoc annotations: https://www.typescriptlang.org/docs/handbook/intro-to-js-ts.html