Skip to content

Commit 9a8b0e0

Browse files
authored
Fix signals breaking with certain vanilla select html elements in react environments due to circular references (#1215)
1 parent f92b8d6 commit 9a8b0e0

File tree

3 files changed

+107
-1
lines changed

3 files changed

+107
-1
lines changed

.changeset/friendly-cycles-flow.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@segment/analytics-signals': patch
3+
---
4+
5+
Fix bug where in vanilla React environments, the onChange events would error due to circular references.
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
import { createInteractionSignal } from '../../../../types/factories'
2+
import { SignalEmitter } from '../../../emitter'
3+
import { OnChangeGenerator } from '../change-gen'
4+
5+
describe('OnChangeGenerator', () => {
6+
let onChangeGenerator: OnChangeGenerator
7+
let emitter: SignalEmitter
8+
let unregister: () => void
9+
beforeEach(() => {
10+
onChangeGenerator = new OnChangeGenerator()
11+
emitter = new SignalEmitter()
12+
})
13+
14+
afterEach(() => {
15+
unregister()
16+
})
17+
18+
it('should emit a signal on change event', async () => {
19+
const emitSpy = jest.spyOn(emitter, 'emit')
20+
const target = document.createElement('input')
21+
target.type = 'text'
22+
target.value = 'new value'
23+
24+
const event = new Event('change', { bubbles: true })
25+
Object.defineProperty(event, 'target', { value: target })
26+
27+
unregister = onChangeGenerator.register(emitter)
28+
document.dispatchEvent(event)
29+
30+
expect(emitSpy).toHaveBeenCalledWith(
31+
createInteractionSignal({
32+
eventType: 'change',
33+
listener: 'onchange',
34+
target: expect.any(Object),
35+
change: { value: 'new value' },
36+
})
37+
)
38+
})
39+
40+
it('should not emit a signal for ignored elements', () => {
41+
const emitSpy = jest.spyOn(emitter, 'emit')
42+
const target = document.createElement('input')
43+
target.type = 'password'
44+
45+
const event = new Event('change', { bubbles: true })
46+
Object.defineProperty(event, 'target', { value: target })
47+
48+
unregister = onChangeGenerator.register(emitter)
49+
document.dispatchEvent(event)
50+
51+
expect(emitSpy).not.toHaveBeenCalled()
52+
})
53+
54+
it('should not emit a signal for elements handled by mutation observer', () => {
55+
const emitSpy = jest.spyOn(emitter, 'emit')
56+
const target = document.createElement('input')
57+
target.type = 'text'
58+
target.setAttribute('value', 'initial value')
59+
60+
const event = new Event('change', { bubbles: true })
61+
Object.defineProperty(event, 'target', { value: target })
62+
63+
unregister = onChangeGenerator.register(emitter)
64+
document.dispatchEvent(event)
65+
66+
expect(emitSpy).not.toHaveBeenCalled()
67+
})
68+
69+
it('should emit a signal with selectedOptions for select elements', () => {
70+
const emitSpy = jest.spyOn(emitter, 'emit')
71+
const target = document.createElement('select')
72+
const option1 = document.createElement('option')
73+
option1.value = 'value1'
74+
option1.label = 'label1'
75+
option1.selected = true
76+
const option2 = document.createElement('option')
77+
option2.value = 'value2'
78+
option2.label = 'label2'
79+
target.append(option1, option2)
80+
81+
const event = new Event('change', { bubbles: true })
82+
Object.defineProperty(event, 'target', { value: target })
83+
84+
unregister = onChangeGenerator.register(emitter)
85+
document.dispatchEvent(event)
86+
87+
expect(emitSpy).toHaveBeenCalledWith(
88+
createInteractionSignal({
89+
eventType: 'change',
90+
listener: 'onchange',
91+
target: expect.any(Object),
92+
change: {
93+
selectedOptions: [{ value: 'value1', label: 'label1' }],
94+
},
95+
})
96+
)
97+
})
98+
})

packages/signals/signals/src/core/signal-generators/dom-gen/change-gen.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,10 @@ export class OnChangeGenerator implements SignalGenerator {
6767
const parseChange = (target: HTMLElement): ChangedEvent | undefined => {
6868
if (target instanceof HTMLSelectElement) {
6969
return {
70-
selectedOptions: Array.from(target.selectedOptions),
70+
selectedOptions: Array.from(target.selectedOptions).map((option) => ({
71+
value: option.value,
72+
label: option.label,
73+
})),
7174
}
7275
}
7376
if (target instanceof HTMLTextAreaElement) {

0 commit comments

Comments
 (0)