[bug] Version 3.1.7 build error for Angular (likely other projects)

[philmayfield]

This appears to be a regression of issue 895.

After updating to version 3.1.7 Angular project fails to run with the error:

Error: export 'sanitize' (imported as 'DOMPurify') was not found in 'dompurify' (possible exports: default)

Background & Context

Using this package in an Angular 18 project, but this is likely an import issue that will affect any project using that syntax.

package.json:
"dompurify: "^3.1.0",

and import like this into the typescript file

import * as DOMPurify from 'dompurify';

Temporary solution :

Use pinned version:
"dompurify: "3.1.6"

[opensourceally]

Faced issue in Angular 14. Used
"dompurify": [
"node_modules/dompurify/dist/purify.min"
]
in tsconfig.json and it is working now.

[jgbpercy]

Er, @jeroen1602 ? Gotta assume this is related to #990 ?

3.1.7 is also broken for me under Angular 17

[cure53]

Hey all, it likely is related as this was the only change we did in this realm for a long time.

As we try to be mostly agnostic towards frameworks and how they utilize DOMPurify, we have no actual idea what Angular 17 needs to be happy again 🙂

That being said, if anyone knows what needs to be done and wants to send a PR, we'd be very happy. As of no, no action is planned but if folks here find a way that works, we will happily merge the PR.

[jeroen1602]

Er, @jeroen1602 ? Gotta assume this is related to #990 ?

3.1.7 is also broken for me under Angular 17

I only tested it against the newer vite compiler, so maybe that is the difference?

[philmayfield]

While I don't pretend to know a ton about what's going on, after some digging this is certainly an issue with using DOMPurify in a TS project, issue rather than being an Angular issue (TS projects and Angular projects just have a lot of overlap).

I believe that the root of the issue is coming from the @types/dompurify package. Specifically around the way it is exported: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/76b4d4a86fee7b95f2ccb90e626f40c95567edeb/types/dompurify/index.d.ts#L4.

Just fiddling with the code locally by changing:

export = DOMPurify;

to

export default DOMPurify;

allows me to import via:

import DOMPurify from 'dompurify';

and things immediately start working.

Like I mentioned I'm a bit out of my element, but the TS linting points to allowSyntheticDefaultImports (docs) being the source, which got me down that path.

[reduckted]

I believe that the root of the issue is coming from the @types/dompurify package.

Just fiddling with the code locally by changing...

I generated type definitions using the TypeScript compiler and it produced the same type of default export, so that looks like it's at least part of the solution.

@cure53 Would you accept a PR that adds the type definitions to this repo by generating them using the TypeScript compiler?

[cure53]

@reduckted Heya - we are happy to accept any PR for this that mitigates the problem and doesn't cause any new or additional breakage.

[cure53]

I recall some related issue from 2022... #713

[cure53]

Closing this for now as it feels that we cannot do much here, workarounds exist and us changing things to address this will cause breakage on other ends.

[reduckted]

@cure53 us changing things to address this will cause breakage on other ends.

What breakage and what other ends are you referring to? If the types are fixed and provided in this package, then nothing else should break.

[cure53]

As far as I can see, that discussion was had once before and whatever path we chose, it was wrong for someone 😅

What benefit would you see if we delivered the types and not DefinitelyTyped?

Or, in other words, why not file a PR against DefinitelyTyped but rather move the responsibility over to the library maintainer and create more effort for them and demote the effort already undertaken by the folks maintaining the types on DefinitelyTyped?

[philmayfield]

What's frustrating is that it wasn't an update to the types that broke things, it was an update to this package.

[cure53]

What's frustrating is that it wasn't an update to the types that broke things, it was an update to this package.

I agree and still have no idea why myself. I mean, we added one line here:

745b521#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R22

And that seemingly messed things up. This is also one reason why I am really hesitant to add more cruft to our package, i.e. bundled types - because there is no way to predict what will happen (for me at least).

[rubenv]

I'm no expert, but you're using the same filename for module and es2020?

[cure53]

cc @jeroen1602 who wrote the PR :D #990

Either way, I am happy to accept PRs that fix this and don't cause new issues, and do a release right after - we have one planned anyway.

Right now I have nothing to work with so nothing will happen from my end. But once I do, I can do the needful 😄

[jeroen1602]

@cure53 you could revert my PR for now since it only fixes a warning and has minimal space saving in the final bundle.

[cure53]

So, the steps would be:

• Remove that line again
• Prepare a new release
• Publish release

Correct? Not gonna happen before at some point next week, but I can do that. @rubenv @philmayfield @reduckted wdyt?

[rubenv]

My gut feeling is that the identical filename causes the wrong thing to end up in the wrong place. Could be as simple as changing that, but I have never worked with this, so could be wrong.

…

On Fri, Oct 11, 2024, 18:17 Cure53 ***@***.***> wrote: So, the steps would be: - Remove that line again - Prepare a new release - Publish release Correct? Not gonna happen before at some point next week, but I can do that. @rubenv <https://github.com/rubenv> @philmayfield <https://github.com/philmayfield> @reduckted <https://github.com/reduckted> wdyt? — Reply to this email directly, view it on GitHub <#1003 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAKPGBENAVPHEHT3V6M2X3Z272ZLAVCNFSM6AAAAABPF7KSXSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBXG4ZTINBUHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[philmayfield]

I would agree that reverting the PR for now makes the most sense - until whatever the underlying issue is can be fully groked.

[reduckted]

So, the steps would be:
...
Correct? Not gonna happen before at some point next week, but I can do that.

@cure53 If you can wait a day or two, I've almost completed adding type definitions to this repository by converting it to TypeScript. There are minimal changes to the actual code (absolutely no logic changes), and no changes to the existing JavaScript files in the dist folder other than the removal of some JSDoc comments and whitespace.

I still need to test it in various use cases (for example, in an Angular project, etc), but I think this is the best path forward.

[cure53]

So, the steps would be:
...
Correct? Not gonna happen before at some point next week, but I can do that.

@cure53 If you can wait a day or two, I've almost completed adding type definitions to this repository by converting it to TypeScript. There are minimal changes to the actual code (absolutely no logic changes), and no changes to the existing JavaScript files in the dist folder other than the removal of some JSDoc comments and whitespace.

I still need to test it in various use cases (for example, in an Angular project, etc), but I think this is the best path forward.

Cool, thanks - we will wait then 🙂

[philmayfield]

The effort is much appreciated everyone!

[reduckted]

A quick update.

I've had some trouble getting the type declarations correct for all of the different scenarios that arethetypeswrong checks. I thought I had that working, but have just discovered another problem with them. Hopefully I'll get that sorted out in the next day or so.

[reduckted]

Good news! I've made a pull request to add type declarations: #1006.

[cure53]

Amazing, thanks a ton, this looks great 😄

@philmayfield does this change do the trick for you as well?

[philmayfield]

I'll give it a shot in the next day or so and let you know! 🙇

[philmayfield]

Hey there, just thought I'd check in as I finally had a chance to sit down and try the fix out. Unfortunately for me its still having an issue, but I'm convinced it's something to do with our main project config. Possibly a compatibility thing to work with commonjs, not totally sure yet. But! When I create a new Angular project and install @reduckted s fix version, it compiles fine and the same component we use on the main project works fine with a slight adjustment to the import syntax.

import DOMPurify from 'dompurify';

as opposed to

import * as DOMPurify from 'dompurify';

So I'm confident in saying that it's a me problem now 😜 - thank you @reduckted for your hard work!

[cure53]

Alright, we are live :) https://github.com/cure53/DOMPurify/releases/tag/3.2.0

[jeroen1602]

I had to add a few things to my tsconfig.json to get it to play nice.

{
  "compilerOptions": {
     "paths": {
       "dompurify": ["node_modules/dompurify/dist/purify.es.d.mts"],
      },
      "types": ["trusted-types"],
  }
}

As well as installing @types/trusted-types as a dev-dependency (I used version ^2.0.7)

After a bit more research:

The types are need if you have skipLibCheck set to false in your tsconfig.lib.json. (This is the default setting I believe).

And the paths part is needed for ngpackgr if you use it in a library. It has its own logic for module resolution and will try to load the purify.cjs.d.ts version and then complain that it is commonjs.

[reduckted]

And the paths part is needed for ngpackgr if you use it in a library. It has its own logic for module resolution and will try to load the purify.cjs.d.ts version and then complain that it is commonjs.

That's interesting. Are you able to link to this module resolution code? Maybe there's something that needs to be added to the package.json file for dompurify to make it pick the right one.

[jeroen1602]

I'm trying to recreate it so that I can work back from the error, but now it is happy to compile without specifying the paths variable in the tsconfig.json. So I'm just mostly confused, but happy it works.