Skip to content

Commit 240bae2

Browse files
committed
fix issue with circularity
1 parent 82bc27b commit 240bae2

File tree

7 files changed

+158
-7
lines changed

7 files changed

+158
-7
lines changed

packages/signals/signals-example/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
"@segment/analytics-signals": "workspace:^",
1212
"react": "^18.0.0",
1313
"react-dom": "^18.0.0",
14+
"react-hook-form": "^7.54.2",
1415
"react-router-dom": "^6.23.1"
1516
},
1617
"devDependencies": {

packages/signals/signals-example/src/components/App.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { loadAnalytics } from '../lib/analytics'
44
import { BrowserRouter as Router, Route, Routes, Link } from 'react-router-dom'
55
import { HomePage } from '../pages/Home'
66
import { OtherPage } from '../pages/Other'
7+
import { ReactHookFormPage } from '../pages/ReactHookForm'
78

89
const App: React.FC = () => {
910
useEffect(() => {
@@ -14,11 +15,13 @@ const App: React.FC = () => {
1415
<Router>
1516
<nav>
1617
<Link to="/">Home</Link>
18+
<Link to="/reacthookform">React Hook Form Example</Link>
1719
<Link to="/other">Other</Link>
1820
</nav>
1921
<Routes>
2022
<Route path="/" element={<HomePage />} />
2123
<Route path="/other" element={<OtherPage />} />
24+
<Route path="/reacthookform" element={<ReactHookFormPage />} />
2225
</Routes>
2326
</Router>
2427
)
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
import React from 'react'
2+
import { useForm, SubmitHandler } from 'react-hook-form'
3+
4+
interface IFormInput {
5+
name: string
6+
selectField: string
7+
expectFormError: boolean
8+
}
9+
10+
const ComplexFormWithHookForm = () => {
11+
const {
12+
register,
13+
handleSubmit,
14+
watch,
15+
formState: { errors },
16+
} = useForm<IFormInput>()
17+
const onSubmit: SubmitHandler<IFormInput> = (data) => {
18+
const statusCode = data.expectFormError ? parseInt(data.name, 10) : 200
19+
const formData = {
20+
status: statusCode,
21+
name: data.name,
22+
selectField: data.selectField,
23+
}
24+
console.log('Submitting form:', JSON.stringify(formData))
25+
fetch('/parrot', {
26+
method: 'POST',
27+
headers: {
28+
'Content-Type': 'application/json',
29+
},
30+
body: JSON.stringify(formData),
31+
})
32+
.then((response) => response.json())
33+
.then((data) => {
34+
console.log('Form submitted successfully:', data)
35+
})
36+
.catch((error) => {
37+
console.error('Error submitting form:', error)
38+
})
39+
}
40+
41+
const name = watch('name')
42+
const statusCode = React.useMemo(() => {
43+
try {
44+
const val = parseInt(name, 10)
45+
return val >= 100 && val <= 511 ? val : 400
46+
} catch (err) {
47+
return 400
48+
}
49+
}, [name])
50+
51+
return (
52+
<div>
53+
<button data-custom-attr="some-custom-attribute">Example Button</button>
54+
<form onSubmit={handleSubmit(onSubmit)}>
55+
<div>
56+
<label htmlFor="ex-input-text">Input some text:</label>
57+
<input
58+
id="ex-input-text"
59+
type="text"
60+
{...register('name', { required: true })}
61+
/>
62+
{errors.name && <span>This field is required</span>}
63+
</div>
64+
<div>
65+
<label htmlFor="ex-select">Select an option:</label>
66+
<select
67+
id="ex-select"
68+
{...register('selectField', { required: true })}
69+
>
70+
<option value="" disabled>
71+
Select
72+
</option>
73+
<option value="Option 1">Option 1</option>
74+
<option value="Option 2">Option 2</option>
75+
</select>
76+
{errors.selectField && <span>This field is required</span>}
77+
</div>
78+
<div>
79+
<label htmlFor="ex-checkbox">{`Force submit network status: ${statusCode}`}</label>
80+
<input
81+
id="ex-checkbox"
82+
type="checkbox"
83+
{...register('expectFormError')}
84+
/>
85+
</div>
86+
<button type="submit">Submit via network req</button>
87+
</form>
88+
<button>
89+
<div>
90+
Other Example Button with <h1>Nested Text</h1>
91+
</div>
92+
</button>
93+
</div>
94+
)
95+
}
96+
97+
export default ComplexFormWithHookForm
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import React from 'react'
2+
import ComplexForm from '../components/ComplexReactHookForm'
3+
import { analytics } from '../lib/analytics'
4+
5+
export const ReactHookFormPage: React.FC = () => {
6+
return (
7+
<main>
8+
<h1>Hello, React Hook Form with TypeScript!</h1>
9+
<ComplexForm />
10+
<button onClick={() => analytics.track('manual track call')}>
11+
Send Manual Track Call
12+
</button>
13+
</main>
14+
)
15+
}

packages/signals/signals/src/core/processor/processor.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { logger } from '../../lib/logger'
22
import { Signal } from '@segment/analytics-signals-runtime'
33
import { AnyAnalytics } from '../../types'
4-
import { MethodName, Sandbox } from './sandbox'
4+
import { AnalyticsMethodCalls, MethodName, Sandbox } from './sandbox'
55

66
export class SignalEventProcessor {
77
private sandbox: Sandbox
@@ -12,7 +12,14 @@ export class SignalEventProcessor {
1212
}
1313

1414
async process(signal: Signal, signals: Signal[]) {
15-
const analyticsMethodCalls = await this.sandbox.process(signal, signals)
15+
let analyticsMethodCalls: AnalyticsMethodCalls
16+
try {
17+
analyticsMethodCalls = await this.sandbox.process(signal, signals)
18+
} catch (err) {
19+
// in practice, we should never hit this error, but if we do, we should log it.
20+
console.error('Error processing signal', { signal, signals }, err)
21+
return
22+
}
1623

1724
for (const methodName in analyticsMethodCalls) {
1825
const name = methodName as MethodName

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

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,24 @@ const getReferencedElement = (
130130
return document.getElementById(value) ?? undefined
131131
}
132132

133+
// Fix a bug with react-hook-form where if the field name conflicts with a native attribute name like 'name',
134+
// the attribute will get overridden with the html element itself. This is a workaround to get the attribute value
135+
const parsePropWithAttributeFallback = (
136+
element: HTMLElement,
137+
propName: string
138+
): string | undefined => {
139+
if (!(propName in element)) {
140+
return
141+
}
142+
// @ts-ignore
143+
const n = element[propName]
144+
if (typeof n === 'string') {
145+
return n
146+
} else if (n instanceof HTMLElement) {
147+
return element.getAttribute(propName) || undefined
148+
}
149+
}
150+
133151
export const parseElement = (el: HTMLElement): AnyParsedElement => {
134152
const labels = parseLabels((el as HTMLInputElement).labels)
135153
const labeledBy = getReferencedElement(el, 'aria-labelledby')
@@ -142,15 +160,15 @@ export const parseElement = (el: HTMLElement): AnyParsedElement => {
142160
// adding a bunch of fields that are not on _all_ elements, but are on enough that it's useful to have them here.
143161
attributes: parseNodeMap(el.attributes),
144162
classList: [...el.classList],
145-
id: el.id,
163+
id: parsePropWithAttributeFallback(el, 'id') || '',
146164
labels,
147165
label: labels[0],
148-
name: (el as HTMLInputElement).name,
166+
name: parsePropWithAttributeFallback(el, 'name'),
149167
nodeName: el.nodeName,
150168
tagName: el.tagName,
151-
title: el.title,
152-
type: (el as HTMLInputElement).type,
153-
value: (el as HTMLInputElement).value,
169+
title: parsePropWithAttributeFallback(el, 'title') || '',
170+
type: parsePropWithAttributeFallback(el, 'type'),
171+
value: parsePropWithAttributeFallback(el, 'value'),
154172
textContent: (el.textContent && cleanText(el.textContent)) ?? undefined,
155173
innerText: (el.innerText && cleanText(el.innerText)) ?? undefined,
156174
describedBy: (describedBy && parseToLabel(describedBy)) ?? undefined,

yarn.lock

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3058,6 +3058,7 @@ __metadata:
30583058
html-webpack-plugin: ^5.6.0
30593059
react: ^18.0.0
30603060
react-dom: ^18.0.0
3061+
react-hook-form: ^7.54.2
30613062
react-router-dom: ^6.23.1
30623063
style-loader: ^4.0.0
30633064
typescript: ^4.7.0
@@ -21145,6 +21146,15 @@ __metadata:
2114521146
languageName: node
2114621147
linkType: hard
2114721148

21149+
"react-hook-form@npm:^7.54.2":
21150+
version: 7.54.2
21151+
resolution: "react-hook-form@npm:7.54.2"
21152+
peerDependencies:
21153+
react: ^16.8.0 || ^17 || ^18 || ^19
21154+
checksum: 49a867ece9894dca82f6552e5eefd012b7db962c56a7543f9275ae0b6ec202d549973c3694e7f97436afc2396549cb8fc8777241dd660b71793547aa9c8e5686
21155+
languageName: node
21156+
linkType: hard
21157+
2114821158
"react-is@npm:^16.13.1":
2114921159
version: 16.13.1
2115021160
resolution: "react-is@npm:16.13.1"

0 commit comments

Comments
 (0)