Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .agents/conventions.md
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,29 @@ For the full automation roster and workflow details, see `.ona/skills/developmen
- No `as` casts unless unavoidable (add a comment explaining why)
- Prefer interfaces for object shapes, types for unions/intersections

### DOM event target safety

`MouseEvent.target`, `MouseEvent.relatedTarget`, and `FocusEvent.relatedTarget` are
typed as `EventTarget | null`. They can be non-Node objects (e.g. when the mouse
leaves to a cross-origin iframe or the browser window). Never cast them with
`as Node` or `as HTMLElement` — use `instanceof` guards instead.

```typescript
// ✅ Correct — instanceof guard before DOM API call
const target = event.relatedTarget;
if (target instanceof Node && container.contains(target)) { ... }

// ✅ Correct — instanceof guard before property access
const target = event.target;
if (!(target instanceof HTMLElement)) return;
target.closest(".menu");

// ❌ Wrong — unsafe cast, throws TypeError if target is not a Node
container.contains(event.target as Node)
```

A static analysis test (`node-contains-safety.test.ts`) enforces this convention.

## shadcn/ui (base-nova style)

This project uses shadcn/ui v4 with the `base-nova` style, which uses `@base-ui/react`
Expand Down
14 changes: 9 additions & 5 deletions src/components/editor/draggable-block-plugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,8 @@ export function DraggableBlockPlugin({
// Track the hovered block element for showing the drag handle
const handleMouseMove = useCallback(
(event: MouseEvent) => {
const target = event.target as HTMLElement;
const target = event.target;
if (!(target instanceof HTMLElement)) return;
if (isOnMenu(target) || isDraggingRef.current) return;

const blockElem = getBlockElement(anchorElem, editor, event);
Expand All @@ -182,8 +183,9 @@ export function DraggableBlockPlugin({
const handleMouseLeave = useCallback((event: MouseEvent) => {
if (menuRef.current && !isDraggingRef.current) {
// Don't hide if the mouse moved to the drag handle itself
const relatedTarget = event.relatedTarget as HTMLElement | null;
if (relatedTarget && isOnMenu(relatedTarget)) return;
const relatedTarget = event.relatedTarget;
if (relatedTarget instanceof HTMLElement && isOnMenu(relatedTarget))
return;

menuRef.current.style.opacity = "0";
targetBlockElemRef.current = null;
Expand All @@ -203,8 +205,10 @@ export function DraggableBlockPlugin({
(event: React.MouseEvent) => {
if (isDraggingRef.current) return;
// Don't hide if the mouse moved back into the anchor element
const relatedTarget = event.relatedTarget as HTMLElement | null;
if (relatedTarget && anchorElem.contains(relatedTarget)) return;
// relatedTarget can be a non-Node EventTarget (e.g. cross-origin iframe)
const relatedTarget = event.relatedTarget;
if (relatedTarget instanceof Node && anchorElem.contains(relatedTarget))
return;

if (menuRef.current) {
menuRef.current.style.opacity = "0";
Expand Down
70 changes: 70 additions & 0 deletions src/components/editor/node-contains-safety.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { describe, it, expect } from "vitest";
import { readFileSync, readdirSync, statSync } from "fs";
import { resolve, join } from "path";

/**
* Regression tests for issue #101: TypeError in Node.contains() from event handlers.
*
* MouseEvent.relatedTarget and Event.target are typed as EventTarget | null,
* which can be a non-Node object (e.g. when mouse leaves to a cross-origin
* iframe). Casting these to Node/HTMLElement and passing them to Node.contains()
* or Element.closest() throws a TypeError.
*
* These tests scan source files for unsafe `as Node` or `as HTMLElement` casts
* on event target properties, ensuring instanceof guards are used instead.
*/

/** Recursively collect .ts/.tsx files under a directory */
function collectSourceFiles(dir: string): string[] {
const files: string[] = [];
for (const entry of readdirSync(dir)) {
const full = join(dir, entry);
if (statSync(full).isDirectory()) {
files.push(...collectSourceFiles(full));
} else if (/\.tsx?$/.test(entry) && !entry.includes(".test.")) {
files.push(full);
}
}
return files;
}

/**
* Matches patterns like:
* event.target as Node
* event.relatedTarget as HTMLElement
* e.target as HTMLElement | null
* event.relatedTarget as HTMLElement | null
*
* These are unsafe because EventTarget is not necessarily a Node.
*/
const UNSAFE_EVENT_TARGET_CAST =
/\.\s*(?:target|relatedTarget)\s+as\s+(?:Node|HTMLElement)(?:\s*\|\s*null)?/g;

describe("DOM API type safety — no unsafe event target casts", () => {
const srcRoot = resolve(__dirname, "../..");
const sourceFiles = collectSourceFiles(join(srcRoot, "components"));

it("no files cast event.target or event.relatedTarget as Node/HTMLElement", () => {
const violations: string[] = [];

for (const filePath of sourceFiles) {
const content = readFileSync(filePath, "utf-8");
const lines = content.split("\n");

for (let i = 0; i < lines.length; i++) {
const matches = lines[i].match(UNSAFE_EVENT_TARGET_CAST);
if (matches) {
const relative = filePath.replace(srcRoot + "/", "");
violations.push(`${relative}:${i + 1}: ${matches[0].trim()}`);
}
}
}

expect(
violations,
"Use `instanceof Node` or `instanceof HTMLElement` guards instead of `as` casts on event targets. " +
"See issue #101.\n\nViolations:\n" +
violations.join("\n")
).toHaveLength(0);
});
});
5 changes: 4 additions & 1 deletion src/components/sidebar/page-search.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,12 @@ export function PageSearch() {
// Close on click outside
useEffect(() => {
function handleClickOutside(e: MouseEvent) {
// e.target can be a non-Node EventTarget (e.g. cross-origin iframe)
const target = e.target;
if (
containerRef.current &&
!containerRef.current.contains(e.target as Node)
(!(target instanceof Node) ||
!containerRef.current.contains(target))
) {
setOpen(false);
}
Expand Down
Loading