Fix javascript builds now that we're using Workers/Worklets

[kixelated]

Unfortunately, the simple days have come to an end. We were using worklets to capture audio but they were crudely being inlined as base64. Now that we're doing more complicated stuff in Workers and Worklets, it's time to do things properly.

When it comes to shipping a library, we need to produce JS files and associated TS types. There's two tools that I've used to do this:

1. Vite (current): Builds Javascript blobs, involving any post-processing like compiling JSX and producing Worker/Worklet assets. Also run TSC to produce declaration files.
2. TSC (old/new): Produces Javascript files by stripping types.

The reason I switched from TSC to Vite was because of SolidJS and JSX. If you don't compile the JSX away, then importing libraries MUST set jsx and jsxFactory settings to match SolidJS which is just incompatible with React.

However, using Vite introduces some new problems, because it produces worklets/workets in an assets folder. An importer of the library would have to serve node_modules/@kixelated/hang/assets as /assets or have some way to configure the path. It's not the end of the world, but it's annoying.

By reverting back to not compiling with a bundler, we can ship a library and rely on the application's bundler. This might require extra configuration but if we keep things simple, it should work out of the box with vite/webpack/parcel etc. They become responsible for building workers and worklets and performing any tree-shaking.

The only snag was the limited usage of JSX in the repo. This has been replaced with vanilla JS, using a tiny helper I created and my signals library. Personally, I think it's muuuuch nicer than SolidJS because it doesn't force functional programming. There's no magic, just append/remove DOM elements based on reactive signals. It should be just as fast, and faster than React of course.

[claude]

Claude finished @kixelated's task —— View job

---

PR Review Complete

Reviewed this pull request for bugs and security issues. Here are my findings:

Tasks:

• Analyze build system changes (Vite → TSC)
• Review new DOM helper implementation
• Check Worker/Worklet handling security
• Examine JSX → vanilla JS conversion
• Look for potential vulnerabilities
• Verify type safety is maintained

Issues Found:

🐛 Critical Bug in DOM Helper - js/signals/src/dom.ts:90

effect.cleanup(() => parent.removeChild(fragment));

This will fail at runtime because DocumentFragment nodes are automatically moved to the parent when appended. The fragment becomes empty and calling removeChild(fragment) will throw an error. Should track and remove the individual child elements instead.

⚠️ Security Concerns:

1. XSS Risk via innerHTML - Multiple locations use innerHTML with string literals containing HTML entities:

   • js/hang/src/publish/element.ts:277-291
   • js/hang/src/watch/element.ts:322-334
   • js/hang/src/support/element.ts:190-194, 270, 275, 317

   While currently using static strings, the DOM helper accepts innerHTML as a parameter (js/signals/src/dom.ts:14) without sanitization. This could enable XSS if user-controlled data is passed.

2. Unsafe Attribute Assignment - js/signals/src/dom.ts:51

   element.setAttribute(key, value);

   No validation on attribute names/values, potentially allowing dangerous attributes like onclick, onload, etc.

Recommendations:

1. Fix the DocumentFragment cleanup bug
2. Remove innerHTML option from DOM helper or add sanitization
3. Validate/sanitize attribute assignments
4. Consider using textContent instead of innerHTML for static status messages

Build System Changes:

✅ The move from Vite bundling to TSC compilation looks correct. Worker/Worklet creation using import.meta.url is secure and appropriate.

[claude]

Claude finished @kixelated's task —— View job

---

PR Security & Bug Review

Tasks:

• Set up development environment
• Review new DOM helper implementation
• Check Worker/Worklet security changes
• Examine JSX → vanilla JS conversions
• Verify if previous critical issues were addressed
• Look for new vulnerabilities or bugs

Issues Found:

🐛 Critical Bug Still Present in DOM Helper - js/signals/src/dom.ts:90

effect.cleanup(() => parent.removeChild(fragment));

This bug from the previous review is still unfixed. When DocumentFragment is appended to a parent, the fragment becomes empty as its children are moved. Calling removeChild(fragment) will throw an error since the fragment is no longer a child of the parent.

⚠️ Security Concerns Still Present:

1. XSS Risk via innerHTML - Multiple locations still use innerHTML with static strings:

   • js/hang/src/publish/element.ts:277-291
   • js/hang/src/watch/element.ts:322-334
   • js/hang/src/support/element.ts:190-194, 270, 275, 317

   The DOM helper accepts innerHTML as a parameter (js/signals/src/dom.ts:14) without sanitization, creating potential XSS vectors if user data is passed.

2. Unsafe Attribute Assignment - js/signals/src/dom.ts:51

   element.setAttribute(key, value);

   No validation on attribute names/values, potentially allowing dangerous attributes.

Recommendations:

1. Fix the DocumentFragment cleanup bug - Track and remove the actual child elements instead of the fragment
2. Remove innerHTML option from DOM helper or add proper sanitization
3. Validate attribute assignments to prevent dangerous attributes like onclick
4. Use textContent instead of innerHTML for static status messages

Build System Analysis:

✅ The move from Vite bundling to TSC compilation looks correct. Worker/Worklet creation using import.meta.url is secure and follows modern standards.

✅ The Worker implementations (vad.ts, transcribe.ts) properly handle message passing and error cases without security issues.

Verdict:

❌ Cannot approve - Critical runtime bug and security concerns need to be addressed before this can be safely merged.