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

Rewrite v-sanitize into vue3/nuxt3 #284

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

truesteps
Copy link
Contributor

@truesteps truesteps commented Jul 7, 2022

Closed: #265

Hey!

I started working on the rewrite, couldn't test the directive yet, but the $sanitize function should already work. I ran into issues running sanitize-html under Vite. A possible solution would be to replace sanitize-html with dompurify.

Are you open to that idea, should I prototype up a rewrite with dompurify? You can check it our here https://github.com/cure53/DOMPurify

ToDo

  • test directive in vue 3
  • write tests
  • test / update nuxt implementation for nuxt 3
  • add typings extending vue
  • fix compatibility issues with Vite and sanitize-html
  • add editorconfig or eslint or any other form of keeping codestyle consistent

@chantouchsek chantouchsek self-requested a review July 8, 2022 11:12
@chantouchsek chantouchsek added the enhancement New feature or request label Jul 8, 2022
@chantouchsek
Copy link
Owner

Yes, it's nice, let go ahead try that to see if it works fine, we can release it for vue 3 and nuxt 3.

@chantouchsek chantouchsek changed the title [WIP] Rewrite v-sanitize into vue 3 Rewrite v-sanitize into vue3/nuxt3 Jul 8, 2022
@chantouchsek
Copy link
Owner

But the another plugin already exist here: https://github.com/LeSuisse/vue-dompurify-html, so better avoid creating the same thing.

@truesteps
Copy link
Contributor Author

@chantouchsek I use the package you linked as a replacement for v-sanitize, 'till I update the implementation :D. The problem with thatone is, that it only implements a directive, there is no sanitize function passed to the context as well as no native nuxt implementation.

@chantouchsek
Copy link
Owner

@chantouchsek I use the package you linked as a replacement for v-sanitize, 'till I update the implementation :D. The problem with thatone is, that it only implements a directive, there is no sanitize function passed to the context as well as no native nuxt implementation.

Ok, great 👍

@truesteps
Copy link
Contributor Author

truesteps commented Oct 29, 2022

@chantouchsek heya! Please could you look into my tsconfig and sanitize.ts file? I am pretty new to TS and I don't know where in the documentation to look for stuff like extending vue or nuxt types so they include $sanitize and stuff like that, so I don't know where to add that in the nuxt3/vue3 version.

I added some basic tests for now and an example environment, where you can run the package in dev mode so you can test it properly without the need of another vue app.

Could you please checkut my version and maybe throw a PR with proper TS settings? After that I'd love to finish writing more tests and giving you the code for PR.

Thanks.

// Edit:

I'll make sure to make a better example in the future :D now it's just for testing while I update the package.
Btw I kept the sanitize-html, since the issue I mentioned seems to be an issue of PostCSS not sanitize-html

@chantouchsek
Copy link
Owner

@truesteps what editor/IDE are you using, that seem, you made to lots of files that doesn't related, can you check and revert all of the at first, then i will look up into it again. thanks.

@truesteps
Copy link
Contributor Author

truesteps commented Oct 29, 2022

@chantouchsek I'm using phpstorm, I added a .editorconfig and reformated the entire project, feel free to modify the .editorconfig to your liking and I'll reformat it again :)

@truesteps
Copy link
Contributor Author

The idea behind that came from the fact, that usually no 2 developers have the same formatting setup, so I added it to ensure all code will look the same in the future

@chantouchsek
Copy link
Owner

@truesteps no don't use phpstorm to write js or ts, you should use webstorm instead.

@truesteps
Copy link
Contributor Author

truesteps commented Oct 30, 2022

@chantouchsek phpstorm is the same like webstorm, except it has support for PHP, nothing else is different. .editorconfig is supported by all editors not just the IntelliJ family of products from jetbrains

anyway, if you want me to rewert the formatting changes, I'll do that :)

@chantouchsek
Copy link
Owner

@chantouchsek phpstorm is the same like webstorm, except it has support for PHP, nothing else is different. .editorconfig is supported by all editors not just the IntelliJ family of products from jetbrains

anyway, if you want me to rewert the formatting changes, I'll do that :)

No, I don't accept that format.

@truesteps
Copy link
Contributor Author

@chantouchsek phpstorm is the same like webstorm, except it has support for PHP, nothing else is different. .editorconfig is supported by all editors not just the IntelliJ family of products from jetbrains
anyway, if you want me to rewert the formatting changes, I'll do that :)

No, I don't accept that format.

what format do you wish to use? Please define it so I can implement it. Thanks

@chantouchsek
Copy link
Owner

@chantouchsek phpstorm is the same like webstorm, except it has support for PHP, nothing else is different. .editorconfig is supported by all editors not just the IntelliJ family of products from jetbrains
anyway, if you want me to rewert the formatting changes, I'll do that :)

No, I don't accept that format.

what format do you wish to use? Please define it so I can implement it. Thanks

Thanks, the current format is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Vue 3 and Nuxt 3
2 participants