Skip to content

Commit e8edd1f

Browse files
committed
Use more robust showValue instead of JSON.stringify
Fixes crash on DOM refs & other cyclic hook values.
1 parent 38bcdff commit e8edd1f

File tree

9 files changed

+159
-9
lines changed

9 files changed

+159
-9
lines changed

packages/react-hook-tracer/src/hooks/useContext.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ const useContextTraced = <T>(context: Context<T>, traceOptions?: UseContextTrace
2424
const traceOrigin = componentRegistry.registerHook('context', traceOptions?.label)
2525
const componentLabel = componentRegistry.getCurrentComponentLabel()
2626

27-
const show = traceOptions?.show ?? ((context: T) => JSON.stringify(context))
27+
const show = traceOptions?.show ?? util.showValue
2828

2929
const contextValue = React.useContext(context)
3030

packages/react-hook-tracer/src/hooks/useMemo.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ const useMemoTraced = <T>(
3030
): T => {
3131
const traceOrigin = componentRegistry.registerHook('memo', traceOptions?.label)
3232
const componentLabel = componentRegistry.getCurrentComponentLabel()
33-
const show = traceOptions?.show ?? ((value: T) => JSON.stringify(value))
33+
const show = traceOptions?.show ?? util.showValue
3434

3535
const isInitialized = useRef(false)
3636

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
import { act, render, screen } from '@testing-library/react'
2+
3+
import { TraceLog } from '../components/TraceLog'
4+
import { getLogEntries, getPanelTraceOrigins, setupUser } from '../test/util'
5+
import { useRef } from './useRef'
6+
import { useTracer } from './useTracer'
7+
8+
test('handles ref-value update', async () => {
9+
const user = setupUser()
10+
const Test = () => {
11+
const { TracePanel } = useTracer()
12+
const ref = useRef(42)
13+
return (
14+
<div>
15+
<span data-testid="ref">{ref.current}</span>
16+
<input type="button" value="inc" onClick={() => (ref.current += 1)} />
17+
<TracePanel />
18+
<TraceLog />
19+
</div>
20+
)
21+
}
22+
expect(getLogEntries()).toHaveLength(0)
23+
render(<Test />)
24+
act(() => jest.runOnlyPendingTimers())
25+
26+
expect(getLogEntries()).toContainEqual({
27+
componentLabel: 'Test-1',
28+
origin: 'ref',
29+
message: 'init:42',
30+
})
31+
expect(screen.getByTestId('ref').textContent).toBe('42')
32+
expect(getPanelTraceOrigins()).toContain('ref:42')
33+
34+
await user.click(screen.getByText('inc'))
35+
act(() => jest.runOnlyPendingTimers())
36+
37+
expect(getLogEntries()).toContainEqual({
38+
componentLabel: 'Test-1',
39+
origin: 'ref',
40+
message: 'update:43',
41+
})
42+
expect(getPanelTraceOrigins()).toContain('ref:43')
43+
})
44+
45+
test('allows missing ref value', () => {
46+
const Test = () => {
47+
const { TracePanel } = useTracer()
48+
useRef()
49+
return <TracePanel />
50+
}
51+
52+
render(<Test />)
53+
expect(getPanelTraceOrigins()).toContain('ref:undefined')
54+
})
55+
56+
test('supports custom label', () => {
57+
const Test = () => {
58+
const { TracePanel } = useTracer()
59+
useRef(42, { label: 'answer' })
60+
return <TracePanel />
61+
}
62+
63+
render(<Test />)
64+
expect(getPanelTraceOrigins()).toContain('ref«answer»:42')
65+
})
66+
67+
test('supports custom show', () => {
68+
const Test = () => {
69+
const { TracePanel } = useTracer()
70+
useRef({ p: 42 }, { show: (v) => `[${v.p}]` })
71+
return <TracePanel />
72+
}
73+
74+
render(<Test />)
75+
expect(getPanelTraceOrigins()).toContain('ref:[42]')
76+
})
77+
78+
test('can handle DOM element', () => {
79+
const Test = () => {
80+
useTracer()
81+
const ref = useRef(null)
82+
return (
83+
<div ref={ref}>
84+
<TraceLog />
85+
</div>
86+
)
87+
}
88+
89+
render(<Test />)
90+
act(() => jest.runOnlyPendingTimers())
91+
92+
expect(getLogEntries()).toContainEqual({
93+
componentLabel: 'Test-1',
94+
origin: 'ref',
95+
message: 'init:null',
96+
})
97+
expect(getLogEntries()).toContainEqual({
98+
componentLabel: 'Test-1',
99+
origin: 'ref',
100+
message: 'update:[object HTMLDivElement]',
101+
})
102+
})
103+
104+
test('can handle cyclic object', () => {
105+
const cyclicObject: { cycle?: unknown } = {}
106+
cyclicObject.cycle = cyclicObject
107+
108+
const Test = () => {
109+
useTracer()
110+
111+
const ref = useRef<{ cycle?: unknown }>()
112+
ref.current = cyclicObject
113+
return (
114+
<div>
115+
<TraceLog />
116+
</div>
117+
)
118+
}
119+
120+
render(<Test />)
121+
act(() => jest.runOnlyPendingTimers())
122+
123+
expect(getLogEntries()).toContainEqual({
124+
componentLabel: 'Test-1',
125+
origin: 'ref',
126+
message: 'init:undefined',
127+
})
128+
expect(getLogEntries()).toContainEqual({
129+
componentLabel: 'Test-1',
130+
origin: 'ref',
131+
message: 'update:[object Object]',
132+
})
133+
})

packages/react-hook-tracer/src/hooks/useRef.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ const useRefTraced = <T>(
4545
const componentLabel = componentRegistry.getCurrentComponentLabel()
4646
const componentInfo = getCurrentComponentInfo()
4747

48-
const showDefinedValue = traceOptions?.show ?? ((value: T) => JSON.stringify(value))
48+
const showDefinedValue = traceOptions?.show ?? util.showValue
4949

5050
const showValue = util.showWithUndefined(showDefinedValue)
5151

packages/react-hook-tracer/src/hooks/useState.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ test('supports custom label', () => {
7676
expect(getPanelTraceOrigins()).toContain('state«answer»:42')
7777
})
7878

79-
test('supports custom showState', () => {
79+
test('supports custom show', () => {
8080
const Test = () => {
8181
const { TracePanel } = useTracer()
8282
useState({ p: 42 }, { show: (s) => `[${s.p}]` })

packages/react-hook-tracer/src/hooks/useState.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ const useStateTraced = <S>(
3939
const traceOrigin = componentRegistry.registerHook('state', traceOptions?.label)
4040
const componentLabel = componentRegistry.getCurrentComponentLabel()
4141

42-
const showDefinedState = traceOptions?.show ?? ((state: S) => JSON.stringify(state))
42+
const showDefinedState = traceOptions?.show ?? util.showValue
4343

4444
const showState = util.showWithUndefined(showDefinedState)
4545

packages/react-hook-tracer/src/hooks/useTracer.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ const mkShowPropValue: (showProps?: ShowProps) => (propKey: string, propValue: u
1919
return showProp(propValue)
2020
}
2121
}
22-
return util.showPropValue(propKey, propValue)
22+
return util.showValue(propValue)
2323
}
2424

2525
export const useTracer = (

packages/react-hook-tracer/src/test/setup.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import '@testing-library/jest-dom/extend-expect'
22
import { cleanup } from '@testing-library/react'
33

4+
import { clearLog } from '../Tracer'
45
import { resetComponentRegistry } from '../componentRegistry'
56

67
jest.useFakeTimers()
@@ -9,4 +10,5 @@ afterEach(() => {
910
cleanup()
1011
jest.runAllTimers()
1112
resetComponentRegistry()
13+
clearLog()
1214
})

packages/react-hook-tracer/src/util.ts

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,21 +25,36 @@ export const showWithUndefined =
2525
(x: T | undefined) =>
2626
x === undefined ? 'undefined' : show(x)
2727

28-
export const showPropValue = (_propKey: string, value: unknown): string => {
28+
export const showValue = (value: unknown): string => {
2929
switch (typeof value) {
3030
case 'bigint':
3131
case 'boolean':
3232
case 'number':
3333
case 'string':
34-
case 'object':
3534
case 'symbol': {
3635
return JSON.stringify(value)
3736
}
3837
case 'undefined': {
3938
return 'undefined'
4039
}
41-
case 'function':
40+
case 'function': {
4241
return '<function>'
42+
}
43+
case 'object': {
44+
if (value === null) {
45+
return 'null'
46+
}
47+
// For DOM elements, toString is just fine.
48+
if (value instanceof HTMLElement) {
49+
return value.toString()
50+
}
51+
// For other objects, try to stringify first.
52+
try {
53+
return JSON.stringify(value)
54+
} catch (error) {
55+
return value.toString()
56+
}
57+
}
4358
}
4459
}
4560

0 commit comments

Comments
 (0)