Skip to content

Commit de3c8dd

Browse files
committed
fix(devtools): claim the inspect click in the capture phase
The source inspector listened for clicks in the bubble phase, so by the time it ran the page had already acted on the click: React had dispatched its synthetic onClick, a router link had navigated. preventDefault() cancels only the browser's own default action, so none of that could be undone. Worse, any ancestor calling stopPropagation() -- a modal or dropdown that closes on an outside click -- stopped the event before it reached document, so inspecting inside one silently did nothing at all. Claiming the click in the capture phase fixes both. It costs the page nothing: the handler returns immediately unless the inspect hotkey is held over an element carrying data-tsd-source, and it already performs the open-in-editor or copy itself.
1 parent 566d39b commit de3c8dd

3 files changed

Lines changed: 138 additions & 2 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@tanstack/devtools': patch
3+
---
4+
5+
Claim the source inspector's click in the capture phase so inspecting an element no longer also activates it, and still works inside a modal or dropdown that stops click propagation.
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
import { render } from '@solidjs/testing-library'
2+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
3+
import { DevtoolsProvider } from '../context/devtools-context'
4+
import { SourceInspector } from './source-inspector'
5+
6+
const SOURCE = 'src/App.tsx:12:3'
7+
const INSPECT_KEYS = ['Shift', 'Alt', 'Control']
8+
9+
const holdInspectHotkey = () => {
10+
for (const key of INSPECT_KEYS) {
11+
window.dispatchEvent(new KeyboardEvent('keydown', { key }))
12+
}
13+
}
14+
15+
/**
16+
* Puts the pointer over `element` and arms the inspector.
17+
*
18+
* The highlight effect reads the element under the cursor rather than the event
19+
* target, so the position has to be moved and `elementFromPoint` stubbed before
20+
* the hotkey flips the inspector on.
21+
*/
22+
const hoverWithHotkey = (element: Element) => {
23+
hover(element)
24+
holdInspectHotkey()
25+
}
26+
27+
/** jsdom implements no `elementFromPoint`, so it is assigned rather than spied on. */
28+
const hover = (element: Element) => {
29+
document.elementFromPoint = () => element
30+
document.dispatchEvent(
31+
new MouseEvent('mousemove', { clientX: 5, clientY: 5 }),
32+
)
33+
}
34+
35+
describe('SourceInspector', () => {
36+
beforeEach(() => {
37+
localStorage.clear()
38+
// `createElementSize` observes the name tag, and jsdom ships no
39+
// ResizeObserver.
40+
vi.stubGlobal(
41+
'ResizeObserver',
42+
class {
43+
observe() {}
44+
unobserve() {}
45+
disconnect() {}
46+
},
47+
)
48+
vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response()))
49+
})
50+
51+
afterEach(() => {
52+
// The held-keys list is a singleton root shared by every test in the file,
53+
// so a test that leaves the hotkey down arms the next one.
54+
window.dispatchEvent(new Event('blur'))
55+
Reflect.deleteProperty(document, 'elementFromPoint')
56+
vi.restoreAllMocks()
57+
vi.unstubAllGlobals()
58+
})
59+
60+
it('opens the source of an element whose ancestor stops click propagation', async () => {
61+
render(() => (
62+
<DevtoolsProvider>
63+
<SourceInspector />
64+
</DevtoolsProvider>
65+
))
66+
67+
// A modal, a dropdown, a menu: anything that closes on an outside click
68+
// stops propagation, which is enough to hide the click from a listener that
69+
// waits for the bubble phase.
70+
const modal = document.createElement('div')
71+
const target = document.createElement('button')
72+
target.setAttribute('data-tsd-source', SOURCE)
73+
modal.append(target)
74+
document.body.append(modal)
75+
modal.addEventListener('click', (e) => e.stopPropagation())
76+
77+
const activated = vi.fn()
78+
target.addEventListener('click', activated)
79+
80+
hoverWithHotkey(target)
81+
await Promise.resolve()
82+
83+
target.dispatchEvent(new MouseEvent('click', { bubbles: true }))
84+
85+
expect(activated).not.toHaveBeenCalled()
86+
expect(fetch).toHaveBeenCalledOnce()
87+
expect(String(vi.mocked(fetch).mock.calls[0]![0])).toContain(
88+
`__tsd/open-source?source=${encodeURIComponent(SOURCE)}`,
89+
)
90+
91+
modal.remove()
92+
})
93+
94+
it('leaves ordinary clicks alone when the hotkey is not held', async () => {
95+
render(() => (
96+
<DevtoolsProvider>
97+
<SourceInspector />
98+
</DevtoolsProvider>
99+
))
100+
101+
const target = document.createElement('button')
102+
target.setAttribute('data-tsd-source', SOURCE)
103+
document.body.append(target)
104+
105+
const activated = vi.fn()
106+
target.addEventListener('click', activated)
107+
108+
hover(target)
109+
await Promise.resolve()
110+
111+
target.dispatchEvent(new MouseEvent('click', { bubbles: true }))
112+
113+
expect(activated).toHaveBeenCalledOnce()
114+
expect(fetch).not.toHaveBeenCalled()
115+
116+
target.remove()
117+
})
118+
})

‎packages/devtools/src/components/source-inspector.tsx‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,18 @@ export const SourceInspector = () => {
9292
})
9393
})
9494

95-
createEventListener(document, 'click', (e) => {
95+
// Capture phase: an inspect click must not also activate what it landed on.
96+
// In the bubble phase this runs after the framework has already dispatched
97+
// its own click -- React's synthetic `onClick` has fired, a router link has
98+
// navigated -- and `preventDefault()` cannot undo any of that; it only
99+
// cancels the browser's own default action. Worse, an ancestor that calls
100+
// `stopPropagation()` (every modal and dropdown that closes on an outside
101+
// click) means this handler never runs at all, so inspecting inside one
102+
// silently does nothing. Claiming the event first costs the page nothing:
103+
// the handler returns immediately unless the inspect hotkey is held over an
104+
// element carrying `data-tsd-source`, and it already performs the
105+
// open-in-editor or copy itself rather than relying on anything downstream.
106+
const onInspectClick = (e: MouseEvent) => {
96107
if (!highlightState.element) return
97108

98109
// Snapshot the source before any signal writes: setDisabledAfterClick
@@ -117,7 +128,9 @@ export const SourceInspector = () => {
117128
baseUrl,
118129
)
119130
fetch(url).catch(() => {})
120-
})
131+
}
132+
133+
createEventListener(document, 'click', onInspectClick, { capture: true })
121134

122135
const currentElementBoxStyles = createMemo(() => {
123136
if (highlightState.element) {

0 commit comments

Comments
 (0)