-
Notifications
You must be signed in to change notification settings - Fork 324
feat(react-native): autocapture touches and clicks on React Native Web #4643
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 8 commits
22f334b
9a4d47b
987e222
042ed27
568e62c
b9b2da9
8797151
d12d768
16b0fca
6606ddd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| 'posthog-react-native': minor | ||
| --- | ||
|
|
||
| Autocapture touches and clicks on React Native Web (including expo-router on web). Touch events there carry no `_targetInst` and every touch was silently dropped, so the element chain is now resolved by walking up from `e.target` to the nearest node carrying a React fiber. `captureTouches` also registers a capture-phase `click` listener on the document on web, emitted with `$event_type: 'click'`, since browsers fire `touchend` only for touch input (react-native-web's `Pressable` stops propagation, and `Modal` renders outside the provider's subtree). Autocapture no longer lets an exception escape into the host app's event dispatch. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,7 +61,60 @@ export const defaultPostHogLabelProp = 'ph-label' | |
|
|
||
| const captureAttributePrefix = 'data-ph-capture-attribute-' | ||
|
|
||
| export const autocaptureFromTouchEvent = (e: any, posthog: PostHog, options: PostHogAutocaptureOptions = {}): void => { | ||
| // react-native-web internals; skipped only where RNW puts them, so a same-named app component is kept. | ||
| // Verified against RNW 0.20.0 and 0.21.2: createElement wraps in LocaleProvider only when | ||
| // `domProps.dir` is set, so a nested element inside a text ancestor never gets one. Recheck on bump. | ||
| const frameworkInternalLabels = ['LocaleProvider'] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] Could we note the RNW version this was verified against? I checked 0.20.0 —
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Noted in d12d768. I checked 0.21.2 as well and the condition is unchanged, createElement only wraps in LocaleProvider when domProps.dir is set, so the comment now records both versions and says to recheck on a bump. |
||
|
|
||
| const reactFiberKeyPattern = /^__react(Fiber|InternalInstance)\$/ | ||
|
|
||
| // Cycle guard, not a depth policy: real DOM chains null-terminate, a malformed parentNode may not. | ||
| const maxFallbackAncestors = 100 | ||
|
|
||
| // Fires per interaction, so warn once: a persistent failure here silently disables autocapture. | ||
| let warnedCaptureFailure = false | ||
|
|
||
| // react-dom (RN Web) events have no _targetInst; the fiber sits on e.target under a randomised | ||
| // __reactFiber$ key. The clicked node may be a non-React node inside a React subtree, so walk up. | ||
| const getFallbackTargetInstance = (e: any): Element | undefined => { | ||
| let node = e.target | ||
|
|
||
| for (let depth = 0; node && typeof node === 'object' && depth < maxFallbackAncestors; depth++) { | ||
| const key = Object.getOwnPropertyNames(node).find((name) => reactFiberKeyPattern.test(name)) | ||
| if (key) { | ||
| return node[key] | ||
| } | ||
| node = node.parentNode | ||
| } | ||
|
|
||
| return undefined | ||
| } | ||
|
|
||
| // Autocapture must never break the host app: a throw would escape into RN's touch dispatch on | ||
| // native, or the DOM click handler on web. Matches the browser SDK, which guards its equivalent | ||
| // document-level handler (packages/browser/src/autocapture.ts). | ||
| export const autocaptureFromTouchEvent = ( | ||
| e: any, | ||
| posthog: PostHog, | ||
| options: PostHogAutocaptureOptions = {}, | ||
| eventType: 'touch' | 'click' = 'touch' | ||
| ): void => { | ||
| try { | ||
| captureFromEvent(e, posthog, options, eventType) | ||
| } catch (error) { | ||
| if (!warnedCaptureFailure) { | ||
| warnedCaptureFailure = true | ||
| console.warn('PostHog autocapture: capturing the interaction threw:', error) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] Not debug-gated, so this fires once in production for everyone. Fine given warn-once — just noting a systematic failure then goes fully silent, which is the case the comment above is worried about.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Leaving this one as is. Bare console.warn is already the pattern across the package (storage.ts:92, 113, 128, 161 and PostHogProvider.tsx:137), so gating just this call site would make it the odd one out. Routing it through posthog.logger isn't a free swap either. That ships a record to the logs product over the network on every failure, which is heavier than a console line and uncomfortably close to a loop when the thing failing is capture itself. Agreed on the substance though, warn-once means a systematic failure surfaces once and then goes quiet for the life of the bundle. Worth revisiting if we ever grow an internal SDK-error channel that isn't the logs product. |
||
| } | ||
| } | ||
| } | ||
|
|
||
| const captureFromEvent = ( | ||
| e: any, | ||
| posthog: PostHog, | ||
| options: PostHogAutocaptureOptions, | ||
| eventType: 'touch' | 'click' | ||
| ): void => { | ||
| const { | ||
| noCaptureProp = 'ph-no-capture', | ||
| customLabelProp = defaultPostHogLabelProp, | ||
|
|
@@ -70,13 +123,15 @@ export const autocaptureFromTouchEvent = (e: any, posthog: PostHog, options: Pos | |
| propsToCapture = ['style', 'testID', 'accessibilityLabel', customLabelProp, 'children'], | ||
| } = options | ||
|
|
||
| if (!e._targetInst) { | ||
| const nativeInst = e._targetInst | ||
| const targetInst: Element | undefined = nativeInst || getFallbackTargetInstance(e) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Low: No-capture ancestor can be skipped The new web fallback feeds clicks into a traversal that terminates when
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confirmed, and it isn't web-only. The Splitting it into its own PR since it's a native bug as much as a web one. The fix is the same shape as the ownership walk added here, scan the full |
||
| if (!targetInst) { | ||
| return | ||
| } | ||
| const elements: PostHogAutocaptureElement[] = [] | ||
| const autocaptureProperties: Record<string, JsonType> = {} | ||
|
|
||
| let currentInst: Element | undefined = e._targetInst | ||
| let currentInst: Element | undefined = targetInst | ||
|
|
||
| while ( | ||
| currentInst && | ||
|
|
@@ -130,14 +185,19 @@ export const autocaptureFromTouchEvent = (e: any, posthog: PostHog, options: Pos | |
| } | ||
|
|
||
| // Try and find a sensible label | ||
| const label = | ||
| typeof props?.[customLabelProp] !== 'undefined' | ||
| ? `${props[customLabelProp]}` | ||
| : currentInst.elementType?.displayName || currentInst.elementType?.name | ||
| const hasCustomLabel = typeof props?.[customLabelProp] !== 'undefined' | ||
| const label = hasCustomLabel | ||
| ? `${props?.[customLabelProp]}` | ||
| : currentInst.elementType?.displayName || currentInst.elementType?.name | ||
|
|
||
| Object.assign(autocaptureProperties, elAutocaptureProperties) | ||
|
|
||
| if (label && !ignoreLabels.includes(label)) { | ||
| // RNW wraps the touched host node directly, so its internals only ever head the chain; a match | ||
| // further up is the app's own component. A user-set label is never a framework internal. | ||
| const isFrameworkWrapper = | ||
| !nativeInst && !hasCustomLabel && elements.length === 0 && frameworkInternalLabels.includes(label as string) | ||
|
|
||
| if (label && !isFrameworkWrapper && !ignoreLabels.includes(label)) { | ||
| el.tag_name = sanitiseLabel(label) | ||
| elements.push(el) | ||
| } | ||
|
|
@@ -164,10 +224,10 @@ export const autocaptureFromTouchEvent = (e: any, posthog: PostHog, options: Pos | |
| element['tag_name'] = lastLabel | ||
| } | ||
| } | ||
| posthog.autocapture('touch', elements, { | ||
| posthog.autocapture(eventType, elements, { | ||
| ...autocaptureProperties, | ||
| $touch_x: e.nativeEvent.pageX, | ||
| $touch_y: e.nativeEvent.pageY, | ||
| $touch_x: e.nativeEvent?.pageX, | ||
| $touch_y: e.nativeEvent?.pageY, | ||
| }) | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.