-
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 5 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 |
|---|---|---|
| @@ -1,10 +1,11 @@ | ||
| import React, { useCallback, useEffect, useMemo } from 'react' | ||
| import React, { useCallback, useEffect, useMemo, useRef } from 'react' | ||
| import { GestureResponderEvent, StyleProp, View, ViewStyle } from 'react-native' | ||
| import { PostHog, PostHogOptions } from './posthog-rn' | ||
| import { autocaptureFromTouchEvent } from './autocapture' | ||
| import { useNavigationTracker } from './hooks/useNavigationTracker' | ||
| import { PostHogContext } from './PostHogContext' | ||
| import { PostHogAutocaptureOptions } from './types' | ||
| import { isWeb } from './utils' | ||
| import { defaultPostHogLabelProp } from './autocapture' | ||
|
|
||
| /** | ||
|
|
@@ -173,11 +174,38 @@ export const PostHogProvider = ({ | |
| [captureTouches, posthog, autocaptureOptions] | ||
| ) | ||
|
|
||
| // Browsers fire touchend only for touch input, so a mouse never reaches onTouchEndCapture. | ||
| // On web listen for click on the document in the CAPTURE phase, matching the browser SDK: | ||
| // RNW's Pressable calls stopPropagation so a bubble-phase handler never sees presses on a | ||
| // button, and RNW's Modal portals its content to document.body, outside this provider's subtree. | ||
| // Read through a ref so an inline `autocapture` object prop doesn't re-attach on every render. | ||
| const optionsRef = useRef(autocaptureOptions) | ||
| useEffect(() => { | ||
| optionsRef.current = autocaptureOptions | ||
| }, [autocaptureOptions]) | ||
|
|
||
| useEffect(() => { | ||
| // The package targets ESNext without the DOM lib, so reach the document off the global. | ||
| const doc = (globalThis as any)?.document | ||
| if (!isWeb() || !captureTouches || !doc?.addEventListener) { | ||
| return | ||
| } | ||
| const handler = (e: any): void => { | ||
| autocaptureFromTouchEvent({ target: e.target, nativeEvent: e }, posthog, optionsRef.current, 'click') | ||
| } | ||
|
ioannisj marked this conversation as resolved.
|
||
| doc.addEventListener('click', handler, true) | ||
|
veria-ai[bot] marked this conversation as resolved.
Outdated
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. [question] Two sibling providers sharing one client each register their own listener, so one click in one subtree enqueues two
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. Good catch on the spec, I had this filed as a preference rather than a conformance gap. Fixed in 8797151. The document listener is now refcounted per client in a module-level WeakMap, so however many providers mount against one client exactly one listener is installed, and a single interaction enqueues one $autocapture. Two tests came with it: two sibling providers sharing a client produce one event, and the listener only detaches when the last provider unmounts. Both fail against the previous commit, so they're real regression guards rather than passing by accident. Worth being explicit that this fixes the duplication and not the scope. Clicks outside the provider's subtree are still captured, and with two different clients each still gets its own document listener, so the cross-client misrouting greptile raised is untouched. Picking that up separately since it needs a fiber ancestry check rather than a counter. |
||
| return () => doc.removeEventListener('click', handler, true) | ||
| }, [captureTouches, posthog]) | ||
|
|
||
| const captureProps = isWeb() | ||
| ? {} | ||
| : { onTouchEndCapture: captureTouches ? (e: GestureResponderEvent) => onTouch('end', e) : undefined } | ||
|
|
||
| return ( | ||
| <View | ||
| {...{ [phLabelProp]: 'PostHogProvider' }} // Dynamically setting customLabelProp (default: ph-label) | ||
| style={style || { flex: 1 }} | ||
| onTouchEndCapture={captureTouches ? (e) => onTouch('end', e) : undefined} | ||
| {...captureProps} | ||
| > | ||
| <PostHogContext.Provider value={{ client: posthog }}> | ||
| {captureScreens && <PostHogNavigationHook options={autocaptureOptions} client={posthog} />} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,7 +61,58 @@ 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. | ||
| 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 +121,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 +183,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 +222,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, | ||
| }) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -14,7 +14,10 @@ export type PostHogNavigationRef = { | |||||||||||
|
|
||||||||||||
| export type PostHogAutocaptureOptions = { | ||||||||||||
| /** | ||||||||||||
| * Enable autocapture of touch events | ||||||||||||
| * Enable autocapture of touch events. | ||||||||||||
| * | ||||||||||||
| * On React Native Web this also captures `click` events from mouse and trackpad input, | ||||||||||||
| * emitted with `$event_type: 'click'`, since browsers fire `touchend` only for touch input. | ||||||||||||
|
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. [suggestion]
Suggested change
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. Applied verbatim in d12d768. You're right that keyboard activation and programmatic .click() both land here with no pointer behind them, and the old wording denied it. |
||||||||||||
| * | ||||||||||||
| * @default false | ||||||||||||
| */ | ||||||||||||
|
|
||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[question] Document scope means clicks outside the provider's subtree get captured too — a sibling button under a named app component comes back as
elements: ["OutsideAppComponent"], $event_type: "click", where native captures nothing. Probably the right trade for Modal portals, but posthog.com currently says "touch events for children ofPostHogProviderare tracked" — can we update that line in the same batch? (Portals work either way; React keeps the fiber.returnchain intact through them.)Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scoped in 16b0fca. The listener stays on the document so Modal still works, but capture now walks the fiber
.returnchain and drops anything whose ancestry doesn't pass through a mounted provider's root node. SOOutsideAppComponentcase captures nothing now, and a click in one provider's subtree no longer reaches a second provider's client. This was verified in the browser.Tbh I did weigh leaving it to a follow-up, multiple roots with distinct clients is rare I would think, but once the wide behaviour ships people depend on it and narrowing later becomes the breaking change