Skip to content
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

Support for Node and TypeScript #691

Closed
vhscom opened this issue May 27, 2022 · 17 comments
Closed

Support for Node and TypeScript #691

vhscom opened this issue May 27, 2022 · 17 comments

Comments

@vhscom
Copy link

vhscom commented May 27, 2022

Feature

New developers are struggling to use DOMPurify with modern development frameworks. Eventually they stumble on #400 but I'd like to channel 2015 and ask for baked in Node support. And type definitions out of the box like XSS would be good to add as well.

@cure53
Copy link
Owner

cure53 commented May 31, 2022

Heya, I am not sure what to do with this ticket. I am not a Node person, don't know what's hot-or-not right now in this realm etc. - this library is pretty much written for the browser and happens to also work with jsdom.

But.

If we get a juicy PR or alike that actually implements what you need, happy to review and go with it. But, based on the above, I really have nothing actionable. Hope that males sense.

@vhscom
Copy link
Author

vhscom commented Jun 1, 2022

I understand. I'm more browser myself. The two actionable items are:

  1. Support isomorphic rendering in SvelteKit to address the Node.
  2. Integrate the type definitions from @types/dompurify or, perhaps, adorn the public API with JSDoc type hints.

The isomorphic rendering is a step towards ultimately just running in a Web Worker after a Node-based app compilation step. If this library can be made to run in SvelteKit and deploy to Cloudflare Pages, that's two birds with one stone.

For types, a definition file exists but it requires a separate install. If someone creates a PR to add TypeScript type definitions, or possibly just JSDoc type hints, the out of the box developer experience would be much improved.

Here's an example Foss app where such a juicy 🧃 Pull could be tested: https://github.com/vhscom/metaform

Unfortunately, my laptop is on the fritz. But this is a popular lib so I know it would make a good alternative to js-xss if these two items can be tackled.

@vhscom
Copy link
Author

vhscom commented Jun 1, 2022

@kkomelin Would you possibly be up for integrating your SSR changes into the main library?

@cure53
Copy link
Owner

cure53 commented Jun 1, 2022

Cool 😄 I would be fine with integrating those if it can be done without affecting current use-cases (my feeling is it won't so all good).

@kkomelin
Copy link

kkomelin commented Jun 2, 2022

Hi guys,

@vhscom Thanks for your suggestion. It's a good idea.

@cure53 That's great that you're open for such integration. Thanks.

I've created an issue to discuss this idea with my co-maintainer and everyone interested kkomelin/isomorphic-dompurify#124

@cure53
Copy link
Owner

cure53 commented Aug 3, 2022

Hey all, any progress here? :)

@kkomelin
Copy link

kkomelin commented Aug 3, 2022

Hi @cure53,

I have not yet received any response from my co-maintainer but we may not need his confirm actually.

As for the types, I can suggest using TypeScript compiler which can generate typings by JS files.

npx -p typescript tsc src/**/*.js --declaration --allowJs --emitDeclarationOnly --outDir types

Creating .d.ts Files from .js files

@kkomelin
Copy link

kkomelin commented Aug 3, 2022

As for combining dompurify and isomorphic-dompurify, it should not be hard code-wise but it can introduce new issues for dompurify project like this one kkomelin/isomorphic-dompurify#54

@cure53
Copy link
Owner

cure53 commented Aug 9, 2022

@kkomelin I might need some help here to understand the general issue. Or issues. So, basically we have two problems here, correct?

Problem one is that we don't have baked in Node support. That, we could fix by bringing isomorphic-dompurify and DOMPurify together, so people don't need to maintain two dependencies, not just one. Alternatively, we could update the dusty README and simply point people to your project, correct?

Problem two is that we don't natively ship any type definitions, so folks have problems like this one, correct? #705 This we could fix by generating the types ourselves, as you described, correct?

@cure53
Copy link
Owner

cure53 commented Aug 9, 2022

@kkomelin And, if I understood Problem two properly, would we not want to generate type definitions for the dist files rather than for the src files? Just wondering - I am not too experienced with TypeScript and related development frameworks.

npx -p typescript tsc src/**/*.js --declaration --allowJs --emitDeclarationOnly --outDir types

@kkomelin
Copy link

kkomelin commented Aug 9, 2022

@cure53 Thanks for working on this issue and suggesting some solutions.

Problem one is that we don't have baked in Node support. That, we could fix by bringing isomorphic-dompurify and DOMPurify together, so people don't need to maintain two dependencies, not just one. Alternatively, we could update the dusty README and simply point people to your project, correct?

I think you got it right.

Problem two is that we don't natively ship any type definitions, so folks have problems like this one, correct? #705 This we could fix by generating the types ourselves, as you described, correct?

I don't think generating types would be enough to solve #705 but it'd definitely improve the dompurify project in general.

@kkomelin And, if I understood Problem two properly, would we not want to generate type definitions for the dist files rather than for the src files? Just wondering - I am not too experienced with TypeScript and related development frameworks.

It's a matter of experimentation because I usually write TypeScript code right away and don't generate typings by JS files afterwards.

@cure53
Copy link
Owner

cure53 commented Aug 10, 2022

Thanks @kkomelin

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

@kkomelin
Copy link

Thank you very much @cure53 for updating the readme.

As for whether it solves all the problems or not, we need to ask @vhscom

@cure53
Copy link
Owner

cure53 commented Aug 13, 2022

Quick update, we now also generate type definitions ourselves, which appears to solve this issue:

#705 (comment)

Does this also solve the issue described here?

@kkomelin
Copy link

That's great news. Thanks @cure53!

@vhscom Could you please confirm if the solutions introduced by @cure53 address the issues you initially reported?

@cure53
Copy link
Owner

cure53 commented Aug 23, 2022

Closing this for now as I feel we are done here for good :D Please let me know in case anything is missing / was overlooked.

@kkomelin
Copy link

Thank you very much @cure53 for your effort.

@cure53 cure53 closed this as completed Aug 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants