-
-
Notifications
You must be signed in to change notification settings - Fork 749
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
How to import DOMPurify as an ECMAScript module from TypeScript? #705
Comments
+1 If anyone has a smart way to solve this, I'd e more than happy to review and accept a PR. |
cc @tdeekens who might have more experience on the topic or know who to ask about this :) |
For what it's worth, I tried modifying DOMPurify's
If I turn on
and then some stuff about resolving As you can tell, I'm way out of my depth here, but I'd guess it's probably possible to achieve what I want by both changing The problem is that may be a breaking change. Hopefully someone who knows what they're talking about can confirm whether I'm right and whether there's a way to have the best of both worlds here. |
@amdw I have been checking another thread and believe that this might be a first step towards a working fix: I ran the command (slightly modified) and generated the type definitions:
Does that change anything from your end when done locally? The types folder is being created, filled up with the type definitions generated from the dist folder. |
Just a heads-up, we updated the README. Maybe this helps solving some of the issues? https://github.com/cure53/DOMPurify#running-dompurify-on-the-server |
Many thanks for trying here and apologies for the slow reply! I tried generating the types using the command in your comment. For me they don't get resolved unless I put them in the I pushed my code to a branch called google/peoplemath@master...dpmodule From the build logs I can see the
To reproduce this: git clone https://github.com/google/peoplemath.git --branch dpmodule
cd peoplemath
npm install
./dompurify_repro.sh
npx ng build --configuration production | grep dompurify I wish I could tell you why this happens and how to fix it, but unfortunately I have no idea. That's as far as I've got, sorry. |
@amdw thanks a ton for setting up this repo, this is amazing and for the first time allows me to actually debug that problem and understand what is happening :D I think I already came close to a solution, will test some things and get back to you in this thread asap. |
Alright, this should work. Locally, your repo produces an error free build when I do the following changes:
This allows me to build error free and should solve your problem without making any other changes. Is this a valid solution? Alternatively, you can leave the code in the |
I think it should be fair to mark this as done :) Please reopen in case anything still doesn't work. |
Apologies for my very slow response here. Thank you very much for the efforts you have made to solve this. Unfortunately I am not able to reproduce the fix. Firstly, it appears that the additions you made to I tried the following to copy the files into git clone https://github.com/cure53/DOMPurify.git
cd DOMPurify
git checkout 4f62dcd63cdc19167261cb980cdeba9b8ae2d2ca
cd ../peoplemath
cp ../DOMPurify/dist/purify.*.d.ts node_modules/dompurify/dist/
npx ng build --configuration production However I still get the warning:
The only change I made to my project was to remove the https://github.com/google/peoplemath/compare/dprepro Cloning the repro at this branch and doing the above should make it possible to reproduce this warning. I'm also slightly confused by this:
This change to the https://github.com/google/peoplemath/compare/dpmodule Since I can't reproduce the fix and one of the key commits has been reverted, would it be possible to reopen this issue? Many thanks! |
Hi @amdw - yes indeed - we reverted all changes. The fixes worked for some folks (#691, #705 (comment)) and caused trouble for others (#712, #713), so we decided to roll back all changes and to leave the typing to DefinitelyTyped and all build issues to the folks who maintain the respective builds. Reopening the issue might not make sense as I don't think this is a bug on our end and don't have resources available to invest more time into this issue, sorry. If you have any idea how our project can do things better, please do let me know, happy to look into it. But I don't think the error is on our end and I am hesitant to experiment around any further as it might cause bugs for people who are fine right now. |
In case anyone else is tempted by isomorphic-dompurify FYI it will not offer an ESM-compatible way to avoid the Ivy compiler "optimization bailouts" message as of the time of writing.
I hope something will develop here🤞 |
Some JavaScript environments discourage the use of CommonJS modules, because they make it harder for bundlers and minifiers to optimise an application. For example, here is the advice from Angular:
https://angular.io/guide/build#configuring-commonjs-dependencies
I've been trying to import DOMPurify from a TypeScript application using ECMAScript module syntax without depending on a CommonJS module, but I can't find a way to do it.
If I just follow the documentation and say:
Then it works, provided I set
allowSyntheticDefaultImports
totrue
in mytsconfig.json
. But this causes a dependency on the CommonJS module, which is what I'm trying to avoid. I think this is because themain
attribute of DOMPurify'spackage.json
points to the CommonJS module.I can see the DOMPurify build does include a
purify.es.js
file which appears to be an ECMAScript module. So if we could just get TypeScript to import DOMPurify from that file instead of the CommonJS file, things would probably work. I tried doing this:And it actually seems to work, in that it does seem to cause the TypeScript compiler to look for DOMPurify in that file. However, the problem is that the TypeScript compiler cannot find the type declarations, even though
@types/dompurify
is installed in my project:So the problem now is that TypeScript doesn't know where to look for the type definitions for this module, now I'm not importing it using the standard
dompurify
name.What I'm doing at the moment (reference) is,
Which allows me to import the CommonJS module without
allowSyntheticDefaultImports
. Then, I'm addingdompurify
toallowedCommonJsDependencies
in myangular.json
to suppress the compiler warning that I'm depending on a CommonJS module. But that's not very satisfactory.I'm not sure if this is a problem with DOMPurify or
@types/dompurify
, or if there is some kind of TypeScript compiler option or import syntax or something I can use that will make everything just work with the ECMAScript module. But I spent considerable time today poring through TypeScript's modules and module resolution documentation and I couldn't find anything.If someone cleverer than me can find a way to import DOMPurify as an ECMAScript module from TypeScript, it would be great to add this to the DOMPurify README to help users in this situation. If this is in fact impossible then it would be great to consider changes to DOMPurify and/or
@types/dompurify
to make it possible.Thanks very much for any help!
The text was updated successfully, but these errors were encountered: