Skip to content

Commit 054db75

Browse files
mgcreamgcrea
committed
feat(update): minor changes
1 parent 8e46f3c commit 054db75

File tree

3 files changed

+135
-3
lines changed

3 files changed

+135
-3
lines changed

‎.claude/picker-onchange-issue-investigation.md‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
# Picker onChange Issue Investigation
2+
3+
## Problem Description
4+
When using multiple Picker components in a form, changing one picker causes other pickers to revert to their original values. Specifically:
5+
- User changes gender picker from "male" to "female" ✅
6+
- User changes ski level picker from "B" to "I"
7+
- This triggers onChange events that revert gender back to "male" and ski level back to "B" ❌
8+
9+
## Root Cause Analysis
10+
11+
### Key Findings
12+
13+
1. **Form Library Behavior**: The app uses TanStack Form with a custom `useAppForm` hook that wraps SwiftUI components. When any field changes, the form re-renders all fields.
14+
15+
2. **View Tree Rebuilding**: Every time a picker changes:
16+
- React form re-renders and calls `updateChildProps` for the changed picker
17+
- This was triggering `updateSwiftUIView` which rebuilds the entire SwiftUI view tree
18+
- When SwiftUI views are recreated, the `.onChange` modifier fires for all pickers
19+
20+
3. **Event Sequence** (from logs):
21+
```
22+
1. User changes picker A → onChange fires correctly
23+
2. Form re-renders → updateChildProps called for picker A
24+
3. View tree rebuilds → ALL picker views are recreated
25+
4. SwiftUI .onChange modifiers fire for ALL pickers with their current values
26+
5. These spurious onChange events revert the values
27+
```
28+
29+
## Attempted Solutions
30+
31+
### 1. ✅ Prevented unnecessary view tree rebuilds
32+
**File**: `src/contexts/SwiftUIContext.tsx`
33+
- Modified `registerNode` to only increment `nodeRegistryVersion` when structure actually changes
34+
- This should have prevented the view tree from rebuilding on prop updates
35+
- **Result**: Still didn't work, suggesting view rebuilds might be happening elsewhere
36+
37+
### 2. ✅ Moved onChange handling to binding setter
38+
**Files**: `ios/components/Picker/PickerView.swift`, `DatePickerView.swift`, `MultiPickerView.swift`
39+
- Initially moved onChange from `.onChange` modifier to the binding setter
40+
- **Result**: Binding setter was still being called during view rebuilds
41+
42+
### 3. ✅ Added state tracking to prevent duplicate onChange
43+
**Files**: Same as above
44+
- Added `@State private var lastReportedValue` to track the last value sent
45+
- Only fire onChange when value differs from lastReportedValue
46+
- **Result**: Still didn't work, suggesting the issue might be in the React/form layer
47+
48+
### 4. ✅ Conditional prop updates
49+
**File**: `ios/components/Picker/PickerProps.swift`
50+
- Modified `merge()` to only update properties that actually changed
51+
- **Result**: No improvement
52+
53+
## Current State of Code
54+
55+
### Modified Files:
56+
1. `src/contexts/SwiftUIContext.tsx` - Prevents unnecessary nodeRegistryVersion increments
57+
2. `ios/components/Picker/PickerView.swift` - Tracks lastReportedValue
58+
3. `ios/components/DatePicker/DatePickerView.swift` - Tracks lastReportedValue
59+
4. `ios/components/MultiPicker/MultiPickerView.swift` - Tracks lastReportedSelections
60+
5. `ios/components/Picker/PickerProps.swift` - Conditional updates in merge()
61+
6. `ios/components/MultiPicker/MultiPickerProps.swift` - Conditional updates in merge()
62+
7. `ios/components/DatePicker/DatePickerProps.swift` - Conditional updates in merge()
63+
64+
## Suspected Issues to Investigate
65+
66+
### 1. **TanStack Form Integration**
67+
The form might be:
68+
- Resetting all field values when one changes
69+
- Calling onChange for all fields during re-render
70+
- Managing state in a way that causes all fields to update
71+
72+
**Files to check**:
73+
- `/Users/olivier/Projects/skitrust/skitrust-mobile/src/hooks/useAppForm.ts`
74+
- The `createAppField` wrapper might be triggering onChange unnecessarily
75+
76+
### 2. **React Re-rendering Pattern**
77+
The form components might be:
78+
- Unmounting and remounting on each change
79+
- Losing their identity keys causing React to treat them as new components
80+
- Not properly memoized
81+
82+
### 3. **Event Propagation**
83+
Check if:
84+
- The form's `handleChange` is being called for all fields
85+
- There's circular event triggering between React and Native
86+
87+
## Next Steps
88+
89+
1. **Add logging to the React side**:
90+
```typescript
91+
// In useAppForm.ts createAppField wrapper
92+
onChange: (value) => {
93+
console.log(`Field onChange called with value: ${value}`);
94+
field.handleChange(value);
95+
}
96+
```
97+
98+
2. **Check if components are remounting**:
99+
- Add useEffect with cleanup to track mount/unmount
100+
- Check if component keys are stable
101+
102+
3. **Investigate TanStack Form behavior**:
103+
- Check if form is resetting all fields on change
104+
- Look for form validation that might trigger resets
105+
- Check if `handleChange` is called for all fields
106+
107+
4. **Alternative approach**:
108+
- Consider using local state in each picker component
109+
- Only sync with form on blur or explicit submit
110+
- Use uncontrolled components with refs
111+
112+
## Logs for Reference
113+
114+
### Pattern observed:
115+
```
116+
1. Change gender to "female" → works ✅
117+
2. Change ski level to "I" → triggers:
118+
- onChange for ski level with "I" ✅
119+
- onChange for gender with "male" (revert) ❌
120+
- onChange for ski level with "B" (revert) ❌
121+
```
122+
123+
This suggests the form is somehow resetting or re-initializing all field values when any field changes.
124+
125+
## Contact Points
126+
- Form implementation: `/Users/olivier/Projects/skitrust/skitrust-mobile/src/hooks/useAppForm.ts`
127+
- Form usage: `/Users/olivier/Projects/skitrust/skitrust-mobile/src/components/profile/ProfilePersonalFieldset.tsx`
128+
- Screen: `/Users/olivier/Projects/skitrust/skitrust-mobile/src/screens/GetStartedScreen.tsx`

‎example/.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,4 +75,5 @@ yarn-error.log
7575
!.yarn/versions
7676

7777
# Custom
78+
.claude/
7879
*.private.tsx

‎src/native/SwiftUIRootNativeComponent.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
/* eslint-disable @typescript-eslint/consistent-type-definitions */
22
/* eslint-disable @typescript-eslint/no-deprecated */
3-
import type { HostComponent, ViewProps } from "react-native";
3+
import {
4+
codegenNativeCommands,
5+
codegenNativeComponent,
6+
type HostComponent,
7+
type ViewProps,
8+
} from "react-native";
49
import type { DirectEventHandler } from "react-native/Libraries/Types/CodegenTypes";
5-
import codegenNativeCommands from "react-native/Libraries/Utilities/codegenNativeCommands";
6-
import codegenNativeComponent from "react-native/Libraries/Utilities/codegenNativeComponent";
710

811
export type NativeSwiftUIEvent = {
912
name: string;

0 commit comments

Comments
 (0)