fix(flags): support multiple children prop in PostHogFeature (#1516)

* fix(flags): support multiple children prop in PostHogFeature

* cleanup

* return prop spread passthrough

* tweak

* non-optional onClick

react/src/components/PostHogFeature.tsx

@@ -1,6 +1,7 @@
 import { useFeatureFlagPayload, useFeatureFlagVariantKey, usePostHog } from '../hooks'
-import React, { useCallback, useEffect, useRef } from 'react'
+import React, { Children, ReactNode, useCallback, useEffect, useRef } from 'react'
 import { PostHog } from '../context'
+import { isFunction, isNull, isUndefined } from '../utils/type-utils'
 
 export type PostHogFeatureProps = React.HTMLProps<HTMLDivElement> & {
     flag: string
@@ -28,18 +29,18 @@ export function PostHogFeature({
     const shouldTrackInteraction = trackInteraction ?? true
     const shouldTrackView = trackView ?? true
 
-    if (match === undefined || variant === match) {
-        const childNode: React.ReactNode = typeof children === 'function' ? children(payload) : children
+    if (isUndefined(match) || variant === match) {
+        const childNode: React.ReactNode = isFunction(children) ? children(payload) : children
         return (
-            <VisibilityAndClickTracker
+            <VisibilityAndClickTrackers
                 flag={flag}
                 options={visibilityObserverOptions}
                 trackInteraction={shouldTrackInteraction}
                 trackView={shouldTrackView}
                 {...props}
             >
                 {childNode}
-            </VisibilityAndClickTracker>
+            </VisibilityAndClickTrackers>
         )
     }
     return <>{fallback}</>
@@ -56,38 +57,24 @@ function captureFeatureView(flag: string, posthog: PostHog) {
 function VisibilityAndClickTracker({
     flag,
     children,
-    trackInteraction,
+    onIntersect,
+    onClick,
     trackView,
     options,
     ...props
 }: {
     flag: string
     children: React.ReactNode
-    trackInteraction: boolean
+    onIntersect: (entry: IntersectionObserverEntry) => void
+    onClick: () => void
     trackView: boolean
     options?: IntersectionObserverInit
 }): JSX.Element {
     const ref = useRef<HTMLDivElement>(null)
     const posthog = usePostHog()
-    const visibilityTrackedRef = useRef(false)
-    const clickTrackedRef = useRef(false)
-
-    const cachedOnClick = useCallback(() => {
-        if (!clickTrackedRef.current && trackInteraction) {
-            captureFeatureInteraction(flag, posthog)
-            clickTrackedRef.current = true
-        }
-    }, [flag, posthog, trackInteraction])
 
     useEffect(() => {
-        if (ref.current === null || !trackView) return
-
-        const onIntersect = (entry: IntersectionObserverEntry) => {
-            if (!visibilityTrackedRef.current && entry.isIntersecting) {
-                captureFeatureView(flag, posthog)
-                visibilityTrackedRef.current = true
-            }
-        }
+        if (isNull(ref.current) || !trackView) return
 
         // eslint-disable-next-line compat/compat
         const observer = new IntersectionObserver(([entry]) => onIntersect(entry), {
@@ -96,11 +83,61 @@ function VisibilityAndClickTracker({
         })
         observer.observe(ref.current)
         return () => observer.disconnect()
-    }, [flag, options, posthog, ref, trackView])
+    }, [flag, options, posthog, ref, trackView, onIntersect])
 
     return (
-        <div ref={ref} {...props} onClick={cachedOnClick}>
+        <div ref={ref} {...props} onClick={onClick}>
             {children}
         </div>
     )
 }
+
+function VisibilityAndClickTrackers({
+    flag,
+    children,
+    trackInteraction,
+    trackView,
+    options,
+    ...props
+}: {
+    flag: string
+    children: React.ReactNode
+    trackInteraction: boolean
+    trackView: boolean
+    options?: IntersectionObserverInit
+}): JSX.Element {
+    const clickTrackedRef = useRef(false)
+    const visibilityTrackedRef = useRef(false)
+    const posthog = usePostHog()
+
+    const cachedOnClick = useCallback(() => {
+        if (!clickTrackedRef.current && trackInteraction) {
+            captureFeatureInteraction(flag, posthog)
+            clickTrackedRef.current = true
+        }
+    }, [flag, posthog, trackInteraction])
+
+    const onIntersect = (entry: IntersectionObserverEntry) => {
+        if (!visibilityTrackedRef.current && entry.isIntersecting) {
+            captureFeatureView(flag, posthog)
+            visibilityTrackedRef.current = true
+        }
+    }
+
+    const trackedChildren = Children.map(children, (child: ReactNode) => {
+        return (
+            <VisibilityAndClickTracker
+                flag={flag}
+                onClick={cachedOnClick}
+                onIntersect={onIntersect}
+                trackView={trackView}
+                options={options}
+                {...props}
+            >
+                {child}
+            </VisibilityAndClickTracker>
+        )
+    })
+
+    return <>{trackedChildren}</>
+}

react/src/components/__tests__/PostHogFeature.test.jsx

@@ -1,7 +1,7 @@
 import * as React from 'react'
-import { useState } from 'react';
+import { useState } from 'react'
 import { render, screen, fireEvent } from '@testing-library/react'
-import { PostHogContext, PostHogProvider } from '../../context'
+import { PostHogProvider } from '../../context'
 import { PostHogFeature } from '../'
 import '@testing-library/jest-dom'
 
@@ -89,8 +89,34 @@ describe('PostHogFeature component', () => {
         expect(given.posthog.capture).toHaveBeenCalledTimes(1)
     })
 
+    it('should track an interaction with each child node of the feature component', () => {
+        given(
+            'render',
+            () => () =>
+                render(
+                    <PostHogProvider client={given.posthog}>
+                        <PostHogFeature flag={given.featureFlag} match={given.matchValue}>
+                            <div data-testid="helloDiv">Hello</div>
+                            <div data-testid="worldDiv">World!</div>
+                        </PostHogFeature>
+                    </PostHogProvider>
+                )
+        )
+        given.render()
+
+        fireEvent.click(screen.getByTestId('helloDiv'))
+        fireEvent.click(screen.getByTestId('helloDiv'))
+        fireEvent.click(screen.getByTestId('worldDiv'))
+        fireEvent.click(screen.getByTestId('worldDiv'))
+        fireEvent.click(screen.getByTestId('worldDiv'))
+        expect(given.posthog.capture).toHaveBeenCalledWith('$feature_interaction', {
+            feature_flag: 'test',
+            $set: { '$feature_interaction/test': true },
+        })
+        expect(given.posthog.capture).toHaveBeenCalledTimes(1)
+    })
+
     it('should not fire events when interaction is disabled', () => {
-
         given(
             'render',
             () => () =>
@@ -114,14 +140,24 @@ describe('PostHogFeature component', () => {
     })
 
     it('should fire events when interaction is disabled but re-enabled after', () => {
-
         const DynamicUpdateComponent = () => {
             const [trackInteraction, setTrackInteraction] = useState(false)
 
             return (
                 <>
-                    <div data-testid="clicker" onClick={() => {setTrackInteraction(true)}}>Click me</div>
-                    <PostHogFeature flag={given.featureFlag} match={given.matchValue} trackInteraction={trackInteraction}>
+                    <div
+                        data-testid="clicker"
+                        onClick={() => {
+                            setTrackInteraction(true)
+                        }}
+                    >
+                        Click me
+                    </div>
+                    <PostHogFeature
+                        flag={given.featureFlag}
+                        match={given.matchValue}
+                        trackInteraction={trackInteraction}
+                    >
                         <div data-testid="helloDiv">Hello</div>
                     </PostHogFeature>
                 </>

react/src/utils/type-utils.ts

@@ -0,0 +1,14 @@
+// from a comment on http://dbj.org/dbj/?p=286
+// fails on only one very rare and deliberate custom object:
+// let bomb = { toString : undefined, valueOf: function(o) { return "function BOMBA!"; }};
+export const isFunction = function (f: any): f is (...args: any[]) => any {
+    // eslint-disable-next-line posthog-js/no-direct-function-check
+    return typeof f === 'function'
+}
+export const isUndefined = function (x: unknown): x is undefined {
+    return x === void 0
+}
+export const isNull = function (x: unknown): x is null {
+    // eslint-disable-next-line posthog-js/no-direct-null-check
+    return x === null
+}