Skip to content

Commit 91dab92

Browse files
authored
Fix react-hook-form circular submit on signals (#1232)
1 parent 9b47033 commit 91dab92

File tree

15 files changed

+608
-196
lines changed

15 files changed

+608
-196
lines changed

.buildkite/pipeline.yml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ steps:
7878
- echo "--- Install dependencies"
7979
- yarn install --immutable
8080
- echo "+++ Run Browser integration tests :pray:"
81-
- yarn turbo run --filter=@internal/browser-integration-tests test
81+
- yarn turbo run --filter=@internal/browser-integration-tests test:int
8282
retry:
8383
automatic:
8484
- exit_status: '*'
@@ -156,6 +156,8 @@ steps:
156156
- yarn turbo run --filter='./packages/signals/*' lint
157157
- echo "+++ Run Tests"
158158
- yarn turbo run --filter='./packages/signals/*' test
159+
- echo "+++ Run Integration Tests"
160+
- yarn turbo run --filter='./packages/signals/*' test:int
159161
plugins:
160162
- ssh://[email protected]/segmentio/cache-buildkite-plugin#v2.0.0:
161163
key: "v1.1-cache-dev-{{ checksum 'yarn.lock' }}"

.changeset/slimy-rabbits-agree.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 circular submit error for react-hook-form

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
"build": "turbo run build --filter='./packages/**'",
2121
"watch": "turbo run watch --filter='./packages/**'",
2222
"dev": "yarn workspace @playground/next-playground run dev",
23-
"prepush": "yarn lint && CI=true yarn test --colors --silent",
23+
"prepush": "turbo run lint --filter='...[master...HEAD]' && CI=true yarn turbo run test --filter='...[master...HEAD]' -- --colors --silent",
2424
"postinstall": "husky install",
2525
"changeset": "changeset",
2626
"update-versions-and-changelogs": "changeset version && yarn version-run-all && bash scripts/update-lockfile.sh",

packages/browser-integration-tests/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
"hoistingLimits": "workspaces"
77
},
88
"scripts": {
9-
"test": "playwright test",
9+
"test:int": "playwright test",
1010
"lint": "yarn concurrently 'yarn:eslint .' 'yarn:tsc --noEmit'",
1111
"concurrently": "yarn run -T concurrently",
1212
"watch:test": "yarn test --watch",

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-integration-tests/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
"scripts": {
99
".": "yarn run -T turbo run --filter=@internal/signals-integration-tests...",
1010
"build": "webpack",
11-
"test": "playwright test",
11+
"test:int": "playwright test",
1212
"test:vanilla": "playwright test src/tests/signals-vanilla",
1313
"test:perf": "playwright test src/tests/performance",
1414
"test:custom": "playwright test src/tests/custom",

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

0 commit comments

Comments
 (0)