Missing type declaration for Typescript

[pmrotule]

First of all, nice work! Using a directive instead of an instance method makes much more sense to me especially because I can keep the Eslint error on v-html. That being said, I had to add types in my project since they were missing in this package.

You could migrate to Typescript or if you want to stick to Javascript, you could also just add JSDoc comments on top of the exported variables. Typescript compiled can generate the declaration file from those comments, we would just need a build script in package.json. Something like vite build && tsc (tsc being the Typescript compiler). What do you think?

I'm happy to contribute a PR, I just want to double-check with you first if you're open to one of the suggestions I've made above.

Cheers ✌️

[leopiccionia]

TypeScript support is one of the key missing goals (another one is a robust test suite). In both cases, the problem is the attempt to support both Vue 2 & 3 for now.

I can see two ways to solve that:

1. adopt different branches (and release channels) for each Vue major version
2. abuse TS structural typing (I need to figure out, for example, breaking changes in the VNode interface)

I'll try the second path on the weekend and report my findings. Anyway, if you have type support figured out, please send a PR.

[pmrotule]

Nice to hear. I figured out the types, but you're right, it is indeed tricky to support both Vue 2 & 3 as the imported types are different. Here's what I got (for Vue 2 in my case):

declare module 'vue-sanitize-directive' {
  import { DirectiveFunction, PluginObject } from 'vue'

  type DirectiveSsrFunction = (binding: Parameters<DirectiveFunction>[1]) => void

  export const directive: {
    getSSRProps: DirectiveSsrFunction
    inserted: DirectiveFunction
    mounted: DirectiveFunction
    update: DirectiveFunction
    updated: DirectiveFunction
  }
  const plugin: PluginObject<{ name?: string }>
  export default plugin
}

Having different branches is a good option. I've also seen vue-jest using a monorepo with a package for each version. The problem is the fact that you need to publish either different packages on npm or use different major versions which can be confusing for Dependabot for instance.

Personally, I would rather go with having the 2 versions in different files, but within the same branch, same package. You could do it by creating aliases for both Vue versions in package.json:

"devDependencies": {
  "vue2": "npm:vue@2.x",
  "vue3": "npm:vue@3.x",
}

then you could add a v2 and v3 subdirectory in your src directory and create a v2 specific and a v3 specific directive.

// src/v2/index.ts

import { DirectiveFunction, PluginObject } from 'vue2'
import sanitizeHtml from 'sanitize-html'

// ...

and add index files pointing to the dist folder:

v2/index.js exporting everything from dist/v2/vue-sanitize-directive.esm.js v2/esm/index.js exporting everything from dist/v2/vue-sanitize-directive.esm.js v2/umd/index.js exporting everything from dist/v2/vue-sanitize-directive.umd.js same for v3 folder

export * from './dist/v2/vue-sanitize-directive.esm'
export { default } from './dist/v2/vue-sanitize-directive.esm'

This way, you can import the version you need (considering that v3 and esm is the default):

import sanitizeDirective from 'vue-sanitize-directive' // v3 esm
import sanitizeDirective from 'vue-sanitize-directive/v2' // v2 esm
import sanitizeDirective from 'vue-sanitize-directive/v2/umd' // v2 umd

What do you think?

[leopiccionia]

I've tried to use the approach of having /vue2 and /vue3 entrypoints. It builds, but doesn't work on runtime (probably, a mistake on externalizing Node dependencies). See #4.

[pmrotule]

@leopiccionia Just a few observations on your PR:

• The official Typescript compiler only supports the exports property of package.json since a few months (since v4.5) so considering that Vite uses Esbuild on development which is in theory still not production-ready, it is possible that the Typescript compiler from Esbuild doesn't support the exports property. Same comment applies to Tsup. I would try to test it after removing /vue3 from your import path (will then use the main property) or try to build it for production which would be compiled with Rollup. Nevertheless, if it really doesn't work with Esbuild, I would try compiling the docs with either Rollup directly or Webpack that uses the official Typescript compiler as far as I know.
• You have a package inside another package in your tree (docs-src instead the root level package) which Typescript might not like. In general, a better structure would be to make your repository a monorepository like I did using Yarn Workspaces here. I'm used to Yarn, but I see you use npm which also as a workspaces feature with similar syntax. So I just moved the files around, but it is broken currently since I still have the same error as you have on build and I switched back to Vite in the directive package without adjusting the entry points. Personally, I would make two different builds for the directive like vite build --config vite.config.vue2.js && vite build --config vite.config.vue3.js.

I hope it helps given you a few more things to try in order to figure out why the build is failing. Let me know if something isn't clear from my observations above.

[leopiccionia]

Even though it's not the cause of the issues, I think I'll eventually address the second point. I've used both PNPM and Lerna workspaces in the past, and I think I can use the current refactor to do it.

The cause of the issues is the following:

1. sanitize-html depends on PostCSS (for parsing <style> tags and style attributes)
2. PostCSS may use some Node built-in packages (like path and url) if available
3. Vite used to properly treeshake those Node dependencies out, but tsup (and new versions of Vite, apparently) doesn't
4. So library builds properly, but fails on runtime when PostCSS tries to "require" those Node built-in packages in the browser during initialization

Apparently, the right fix for PostCSS is aliasing those Node built-in packages to empty files; I just didn't figure out yet how to properly do it in the current build pipeline. If this fails, I may use a PostCSS drop-in replacement for browsers, if I find one.

As an experiment, I want to replace sanitize-html with dompurify (different API, same purpose). I'll probably release it as a different package to avoid breakage.