Merge pull request #3446 from guardian/adblocker-in-commercial-metrics-2

CommercialMetrics includes ad blocker use

dotcom-rendering/src/web/components/CommercialMetrics.tsx

@@ -1,46 +1,42 @@
 import type { ABTest } from '@guardian/ab-core';
 import { tests } from '@frontend/web/experiments/ab-tests';
-import { useEffect, useState } from 'react';
 import { sendCommercialMetrics } from '@guardian/commercial-core';
+import { useOnce } from '@root/src/web/lib/useOnce';
 import { useAB } from '@guardian/ab-react';
 import { useDocumentVisibilityState } from '../lib/useDocumentHidden';
+import { useAdBlockInUse } from '../lib/useAdBlockInUse';
 
+// TODO disallow undefined browserIds by placing conditional in App.tsx
+// so that we wait to render this component until browserId is defined.
 export const CommercialMetrics: React.FC<{
 	pageViewId: string;
 	browserId: string | undefined;
 }> = ({ pageViewId, browserId }) => {
 	const ABTestAPI = useAB();
 	const visibilityState = useDocumentVisibilityState();
 
-	const [sentCommercialMetrics, setSentCommercialMetrics] = useState<boolean>(
-		false,
-	);
-
-	useEffect(() => {
-		if (visibilityState !== 'hidden') return;
-		if (sentCommercialMetrics) return;
+	const adBlockerInUse = useAdBlockInUse();
+	// only send metrics when visibility state changes to hidden;
+	const isHidden = visibilityState === 'hidden' || undefined
 
+	useOnce(() => {
 		const testsToForceMetrics: ABTest[] = [];
-		const shouldForceMetrics = ABTestAPI.allRunnableTests(
-			tests,
-		).some((test) =>
-			testsToForceMetrics.map((t) => t.id).includes(test.id),
+		const shouldForceMetrics = ABTestAPI.allRunnableTests(tests).some(
+			(test) => testsToForceMetrics.map((t) => t.id).includes(test.id),
 		);
 		const userIsInSamplingGroup = Math.random() <= 1 / 100;
 		const isDev =
 			window.guardian.config.page.isDev ||
 			window.location.hostname.includes('localhost');
 
-		if (isDev || shouldForceMetrics || userIsInSamplingGroup)
-			setSentCommercialMetrics(
-				sendCommercialMetrics(pageViewId, browserId, Boolean(isDev)),
-			);
+		if (isDev || shouldForceMetrics || userIsInSamplingGroup) {
+			sendCommercialMetrics(pageViewId, browserId, Boolean(isDev), adBlockerInUse);
+		}
 	}, [
 		ABTestAPI,
 		pageViewId,
-		browserId,
-		visibilityState,
-		sentCommercialMetrics,
+		adBlockerInUse,
+		isHidden
 	]);
 
 	// We don’t render anything

dotcom-rendering/src/web/components/WhenAdBlockInUse.tsx

@@ -13,7 +13,7 @@ type Props = {
  * detection checks and returns a promise.
  *
  * Sure, you could just use this custom hook directly but sometimes code is easier
- * to read and undertand when conditional rendering is done using composition
+ * to read and understand when conditional rendering is done using composition
  *
  * @param {ReactNode} children - What gets rendered if an ad blocker is in use
  * */

dotcom-rendering/src/web/lib/useAdBlockInUse.ts

@@ -12,9 +12,7 @@ export const useAdBlockInUse = () => {
 	useEffect(() => {
 		// eslint-disable-next-line @typescript-eslint/no-floating-promises
 		isAdBlockInUse().then((blockerDetected) => {
-			// We're using react state here to trigger the rerender, but only
-			// if a blocker was detected
-			if (blockerDetected) setIsInUse(true);
+			setIsInUse(blockerDetected);
 		});
 	}, []);