Skip to content

Improve Web Worker compatibility (partial)#242

Merged
kkomelin merged 1 commit intokkomelin:masterfrom
wSedlacek:feat/worker-support
Nov 22, 2025
Merged

Improve Web Worker compatibility (partial)#242
kkomelin merged 1 commit intokkomelin:masterfrom
wSedlacek:feat/worker-support

Conversation

@wSedlacek
Copy link
Copy Markdown
Contributor

@wSedlacek wSedlacek commented Dec 28, 2023

Current Behavior

When importing into a worker environment the following runtime error occurs

Uncaught ReferenceError: window is not defined
    at node_modules/.pnpm/isomorphic-dompurify@1.13.0_bufferutil@4.0.8/node_modules/isomorphic-dompurify/browser.js (isomorphic-dompurify.js?v=650fd638:949:22)

Change in behavior

The runtime error does not occur and DOMPurify is accessible both as a export
and on self.DOMPurify/globalThis.DOMPurify in workers

Developer Notes

There are probably a dozen different ways to fix this issue, the one chosen was
picked for being the one with the fewest number of changes while being
backwards compatible all the way back to browser versions from 2012 (Firefox
back to 2004)

See: https://developer.mozilla.org/en-US/docs/Web/API/Window/self#examples

Other approaches considered

  • Using typeof window !== "unefined"
    • This method of determining if the window exists and if this code is
      executing in a worker was considered however after this check was done the
      split in exuection after this was detected would have either needed to use
      self or pollyfil window on self. Since self always works it seemed
      more straight forward to just always use it instead of window

Example:

const globalObject = typeof window !== "undefined" ? window : self;
module.exports = globalObject.DOMPurify || (globalObject.DOMPurify = require('dompurify').default || require('dompurify'));
  • Using globalThis
    • This method would be better if trying to target the same code on server and
      browser however it is a much newer feature and not available on all platforms
      (ie, React Native, etc.)

Example:

module.exports = globalThis.DOMPurify || (globalThis.DOMPurify = require('dompurify').default || require('dompurify'));
  • Using typeof window !== "undefined" but not assigning to self.DOMPurify
    • This would simply remove the accessing of window in worker environments
      effectively killing the polyfil nature of this in worker environments but
      still allowing the import

Example:

const dompurify = require("dompurify")

if (typeof window !== "undefined") {
  window.DOMPurify = window.DOMPurify || dompurify.default || dompurify;
}

module.exports = typeof window !== "undefined" ? window.DOMPurify : dompurify.default || dompurify;
  • Using optional chaning
    • This would have had the same outcome of not having DOMPurify accessible on
      self/globalThis as the previous method however it would rely on a feature
      only aviable in newer JavaScript environments.

Example:

const dompurify = require("dompurify")

if (typeof window !== "undefined") {
  window.DOMPurify = window.DOMPurify || dompurify.default || dompurify;
}

module.exports = window?.DOMPurify || dompurify.default || dompurify;
  • Using null coalescing
    • Again this feature is very new and not supported in every environment
const dompurify = require("dompurify")

if (typeof window !== "undefined") {
  window.DOMPurify ??= dompurify.default || dompurify;
}

module.exports = window?.DOMPurify : dompurify.default || dompurify;

fixes: #240

@kkomelin
Copy link
Copy Markdown
Owner

kkomelin commented Dec 30, 2023

Thank you very much @wSedlacek for such a thoughtful analysis of solutions and choosing the best one for your PR!
Please give me a couple of days to test the chosen solution.

@kkomelin kkomelin added the enhancement New feature or request label Dec 30, 2023
@kkomelin
Copy link
Copy Markdown
Owner

kkomelin commented Dec 30, 2023

@wSedlacek I'm having hard times importing isomorphic-dompurify from inside of a Web Worker script. I don't have much experience with web workers. Could you please instruct me how to do it?

I'm using Vite vanilla template and importing this way import DOMPurify from "isomorphic-dompurify";, but unfortunately it gives me DOMPurify.sanitize is not a function.

In my package.json:

 "dependencies": {
    "isomorphic-dompurify": "github:wSedlacek/isomorphic-dompurify#feat/worker-support"
  }

@wSedlacek
Copy link
Copy Markdown
Contributor Author

I’ll create an example today for testing and get back to you.

@wSedlacek wSedlacek marked this pull request as draft January 2, 2024 17:01
@wSedlacek
Copy link
Copy Markdown
Contributor Author

wSedlacek commented Jan 2, 2024

https://github.com/cure53/DOMPurify/blob/d7498e0546c0f67cf95437ae430ff2035582a689/src/purify.js#L90-L96

So dompurify itself is also checking the window and early exiting. I am going to mark this as a draft for a moment while I evaluate the options.

Update:
I was able to get it running with linkedom as noted in the comments of cure53/DOMPurify#577

import createDOMPurify from "isomorphic-dompurify";
import { parseHTML } from "linkedom";

const { window } = parseHTML("<!DOCTYPE html><body>");
const DOMPurify = createDOMPurify(window);

self.onmessage = (event) => {
  const purified = DOMPurify.sanitize(event.data);
  console.log(`On Worker Thread: ${purified}`); // Should be <img src="x”> but is <img src=x onerror=alert(1)//>
};

The only issue is that while it does run, it doesn’t actually santaize anything at all.
This all said, without the changing in this PR the example doesn’t run inside works at all (because it would crash when trying to use window.DOMPurify on import)
I think it is better to have the issues with web workers be entirity with dompurify instead of the introp layer.

@wSedlacek wSedlacek marked this pull request as ready for review January 2, 2024 17:28
@kkomelin
Copy link
Copy Markdown
Owner

kkomelin commented Jan 3, 2024

Thank you @wSedlacek for your work on this issue! Really great job!

However, I think we cannot release this change without dompurify itself adding support for Web Worker, so I suggest us keeping this issue open in case anything changes in the future. For your particular case, you may consider using alternatives for now.

@kkomelin
Copy link
Copy Markdown
Owner

@coderabbitai wdyt?

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 22, 2025

Tip

For best results, initiate chat on the files or code changes.

@kkomelin, thank you for asking my opinion! This is an interesting technical decision. Here's my analysis:

Arguments for merging this PR:

  1. Partial improvement is valuable - The change prevents an immediate crash when importing the library in Web Workers, which is better than the current state. Developers working in worker contexts won't get a runtime error on import.

  2. Technically sound change - Using self instead of window is a best practice that improves cross-environment compatibility without breaking existing functionality. It's backward compatible back to very old browser versions.

  3. Clear separation of concerns - With this change, the limitation becomes clearly a dompurify issue rather than an isomorphic-dompurify issue. The interop layer works correctly.

Arguments against merging:

  1. Misleading functionality - Users might assume that if the import works, the library is fully functional in workers. This could lead to bugs in production code where sanitization silently fails.

  2. Incomplete solution - The core functionality (sanitization) doesn't work, so it's more of a "doesn't crash" fix rather than "works in workers" fix.

My recommendation:

I'd lean toward merging the PR with clear documentation caveats, because:

  • The change improves the codebase quality regardless of Web Worker support
  • It unblocks developers who might want to use conditional logic or workarounds
  • It positions the library better for when/if DOMPurify adds worker support

However, critical documentation updates are needed:

  • Update README to explicitly state that Web Workers are not fully supported due to DOMPurify limitations
  • Link to the relevant DOMPurify issue (#577)
  • Show the linkedom workaround with clear warnings about its limitations
  • Consider changing the issue title from "fix" to something like "improve Web Worker compatibility (partial)"

This way, you get the benefits of better code without misleading users about capabilities. What do you think?

@kkomelin kkomelin changed the title Web Worker Support Improve Web Worker compatibility (partial) Nov 22, 2025
@kkomelin
Copy link
Copy Markdown
Owner

@coderabbitai Thanks for your input. Would you recommend releasing this change as a major version?

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 22, 2025

Tip

For best results, initiate chat on the files or code changes.

@kkomelin, based on semantic versioning principles, I would recommend this as a minor or patch version release, not a major version. Here's why:

Why not a major version:

  • No breaking changes: Existing code in browser contexts continues to work identically. Since self === window in browsers, self.DOMPurify and window.DOMPurify reference the same object.
  • Backward compatible: All existing use cases remain functional without any code changes required from users.
  • Additive improvement: It enables a new context (Web Workers) to import the library without crashing, even if full functionality requires additional work.

Version recommendation:

I'd suggest a minor version (0.X.0) bump because:

  1. It expands the environments where the library can be imported (new capability, even if limited)
  2. It's a deliberate behavior improvement, not just a bug fix
  3. The PR objectives describe it as "improve Web Worker compatibility" which aligns with a feature enhancement

Alternatively, a patch version (0.0.X) could be justified if you view this purely as fixing a bug (the crash on import).

Important considerations:

  • Document this as a "partial" Web Worker support improvement in the changelog
  • Clearly state that full sanitization functionality in Workers requires DOMPurify upstream changes
  • Update README with the limitations before releasing

Major versions should be reserved for:

  • Actually breaking changes to the API
  • Dropping support for environments/browsers
  • Changes requiring user code modifications

This change doesn't meet any of those criteria, so a major version bump would likely confuse users who expect breaking changes in major releases.

Would you like me to help draft changelog or documentation updates for this release?

@kkomelin kkomelin merged commit 6d06f11 into kkomelin:master Nov 22, 2025
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.

Web Worker Support

2 participants