chore: Connect performance marks to DOM elements (#2345)

cloudscape-design · Jun 7, 2024 · ffcd645 · ffcd645
1 parent 09430c0
commit ffcd645
Show file tree

Hide file tree

Showing 7 changed files with 121 additions and 18 deletions.
diff --git a/src/button/__integ__/performance-marks.test.ts b/src/button/__integ__/performance-marks.test.ts
@@ -5,7 +5,11 @@ import { BasePageObject } from '@cloudscape-design/browser-test-tools/page-objec
 
 function setupTest(
   pageName: string,
-  testFn: (page: BasePageObject, getMarks: () => Promise<PerformanceMark[]>) => Promise<void>
+  testFn: (parameters: {
+    page: BasePageObject;
+    getMarks: () => Promise<PerformanceMark[]>;
+    getElementByPerformanceMark: (id: string) => Promise<WebdriverIO.Element>;
+  }) => Promise<void>
 ) {
   return useBrowser(async browser => {
     const page = new BasePageObject(browser);
@@ -14,14 +18,16 @@ function setupTest(
       const marks = await browser.execute(() => performance.getEntriesByType('mark') as PerformanceMark[]);
       return marks.filter(m => m.detail?.source === 'awsui');
     };
-    await testFn(page, getMarks);
+    const getElementByPerformanceMark = (id: string) => browser.$(`[data-analytics-performance-mark="${id}"]`);
+
+    await testFn({ page, getMarks, getElementByPerformanceMark });
   });
 }
 
 describe('Button', () => {
   test(
     'Emits a mark only for primary visible buttons',
-    setupTest('performance-marks', async (_, getMarks) => {
+    setupTest('performance-marks', async ({ getMarks, getElementByPerformanceMark }) => {
       const marks = await getMarks();
 
       expect(marks).toHaveLength(1);
@@ -33,12 +39,16 @@ describe('Button', () => {
         disabled: false,
         text: 'Primary button',
       });
+
+      expect(await getElementByPerformanceMark(marks[0].detail.instanceIdentifier).then(e => e.getText())).toBe(
+        'Primary button'
+      );
     })
   );
 
   test(
     'Emits a mark when properties change',
-    setupTest('performance-marks', async (page, getMarks) => {
+    setupTest('performance-marks', async ({ page, getMarks, getElementByPerformanceMark }) => {
       await page.click('#disabled');
       await page.click('#loading');
       const marks = await getMarks();
@@ -52,6 +62,11 @@ describe('Button', () => {
         disabled: true,
         text: 'Primary button',
       });
+
+      expect(await getElementByPerformanceMark(marks[1].detail.instanceIdentifier).then(e => e.getText())).toBe(
+        'Primary button'
+      );
+
       expect(marks[2].name).toBe('primaryButtonUpdated');
       expect(marks[2].detail).toMatchObject({
         source: 'awsui',
@@ -60,18 +75,25 @@ describe('Button', () => {
         disabled: true,
         text: 'Primary button',
       });
+
+      expect(await getElementByPerformanceMark(marks[2].detail.instanceIdentifier).then(e => e.getText())).toBe(
+        'Primary button'
+      );
     })
   );
 
   test(
     'Does not emit a mark when inside a modal',
-    setupTest('performance-marks-in-modal', async (page, getMarks) => {
+    setupTest('performance-marks-in-modal', async ({ getMarks, getElementByPerformanceMark }) => {
       const marks = await getMarks();
 
       expect(marks).toHaveLength(1);
       expect(marks[0].detail).toMatchObject({
         text: 'Button OUTSIDE modal',
       });
+      expect(await getElementByPerformanceMark(marks[0].detail.instanceIdentifier).then(e => e.getText())).toBe(
+        'Button OUTSIDE modal'
+      );
     })
   );
 });
diff --git a/src/button/internal.tsx b/src/button/internal.tsx
@@ -86,7 +86,7 @@ export const InternalButton = React.forwardRef(
     const { stepNumber, stepNameSelector } = useFunnelStep();
     const { subStepSelector, subStepNameSelector } = useFunnelSubStep();
 
-    usePerformanceMarks(
+    const performanceMarkAttributes = usePerformanceMarks(
       'primaryButton',
       variant === 'primary',
       buttonRef,
@@ -145,6 +145,7 @@ export const InternalButton = React.forwardRef(
     const buttonProps = {
       ...props,
       ...__nativeAttributes,
+      ...performanceMarkAttributes,
       tabIndex,
       // https://github.com/microsoft/TypeScript/issues/36659
       ref: useMergeRefs(buttonRef, __internalRootRef),

diff --git a/src/internal/hooks/use-performance-marks/__tests__/use-performance-marks.test.tsx b/src/internal/hooks/use-performance-marks/__tests__/use-performance-marks.test.tsx
@@ -0,0 +1,40 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+import React, { useRef } from 'react';
+import { renderToStaticMarkup } from 'react-dom/server';
+import { render } from '@testing-library/react';
+import { usePerformanceMarks } from '../index';
+
+function Demo() {
+  const ref = useRef<HTMLDivElement>(null);
+  const attributes = usePerformanceMarks('test-component', true, ref, () => ({}), []);
+  return <div {...attributes} ref={ref} data-testid="element" />;
+}
+
+describe('Data attribute', () => {
+  test('the attribute should be present after the first render', () => {
+    const { getByTestId } = render(<Demo />);
+
+    expect(getByTestId('element')).toHaveAttribute('data-analytics-performance-mark');
+  });
+
+  test('the attribute should be present after re-rendering', () => {
+    const { getByTestId, rerender } = render(<Demo />);
+
+    const attributeValueBefore = getByTestId('element').getAttribute('data-analytics-performance-mark');
+
+    rerender(<Demo />);
+
+    expect(getByTestId('element')).toHaveAttribute('data-analytics-performance-mark');
+
+    const attributeValueAfter = getByTestId('element').getAttribute('data-analytics-performance-mark');
+
+    expect(attributeValueAfter).toBe(attributeValueBefore);
+  });
+
+  test('should not render the attribute during server-side rendering', () => {
+    const markup = renderToStaticMarkup(<Demo />);
+
+    expect(markup).toBe('<div data-testid="element"></div>');
+  });
+});
diff --git a/src/internal/hooks/use-performance-marks.ts → ...rnal/hooks/use-performance-marks/index.ts b/src/internal/hooks/use-performance-marks.ts → ...rnal/hooks/use-performance-marks/index.ts
@@ -1,20 +1,45 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-import { useEffect } from 'react';
-import { useUniqueId } from './use-unique-id';
-import { useEffectOnUpdate } from './use-effect-on-update';
-import { useModalContext } from '../context/modal-context';
+import { useEffect, useRef } from 'react';
+import { useRandomId } from '../use-unique-id';
+import { useEffectOnUpdate } from '../use-effect-on-update';
+import { useModalContext } from '../../context/modal-context';
 
+/*
+This hook allows setting an HTML attribute after the first render, without rerendering the component.
+*/
+function usePerformanceMarkAttribute(elementRef: React.RefObject<HTMLElement>, value: string) {
+  const attributeName = 'data-analytics-performance-mark';
+
+  const attributeValueRef = useRef<string | undefined>();
+
+  useEffect(() => {
+    // With this effect, we apply the attribute only on the client, to avoid hydration errors.
+    attributeValueRef.current = value;
+    elementRef.current?.setAttribute(attributeName, value);
+  }, [value, elementRef]);
+
+  return {
+    [attributeName]: attributeValueRef.current,
+  };
+}
+
+/**
+ * This function returns an object that needs to be spread onto the same
+ * element as the `elementRef`, so that the data attribute is applied
+ * correctly.
+ */
 export function usePerformanceMarks(
   name: string,
   enabled: boolean,
   elementRef: React.RefObject<HTMLElement>,
   getDetails: () => Record<string, string | boolean | number | undefined>,
   dependencies: React.DependencyList
 ) {
-  const id = useUniqueId();
+  const id = useRandomId();
   const { isInModal } = useModalContext();
+  const attributes = usePerformanceMarkAttribute(elementRef, id);
 
   useEffect(() => {
     if (!enabled || !elementRef.current || isInModal) {
@@ -66,4 +91,6 @@ export function usePerformanceMarks(
     });
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, dependencies);
+
+  return attributes;
 }
diff --git a/src/internal/hooks/use-unique-id/index.ts b/src/internal/hooks/use-unique-id/index.ts
@@ -3,15 +3,15 @@
 import React, { useRef } from 'react';
 
 let counter = 0;
-const useIdFallback = () => {
+export const useRandomId = () => {
   const idRef = useRef<string | null>(null);
   if (!idRef.current) {
     idRef.current = `${counter++}-${Date.now()}-${Math.round(Math.random() * 10000)}`;
   }
   return idRef.current;
 };
 
-const useId: typeof useIdFallback = (React as any).useId ?? useIdFallback;
+const useId: typeof useRandomId = (React as any).useId ?? useRandomId;
 
 export function useUniqueId(prefix?: string) {
   return `${prefix ? prefix : ''}` + useId();

diff --git a/src/table/__integ__/performance-marks.test.ts b/src/table/__integ__/performance-marks.test.ts
@@ -3,22 +3,30 @@
 import useBrowser from '@cloudscape-design/browser-test-tools/use-browser';
 import { BasePageObject } from '@cloudscape-design/browser-test-tools/page-objects';
 
-function setupTest(testFn: (page: BasePageObject, getMarks: () => Promise<PerformanceMark[]>) => Promise<void>) {
+function setupTest(
+  testFn: (parameters: {
+    page: BasePageObject;
+    getMarks: () => Promise<PerformanceMark[]>;
+    getElementByPerformanceMark: (id: string) => Promise<WebdriverIO.Element>;
+  }) => Promise<void>
+) {
   return useBrowser(async browser => {
     const page = new BasePageObject(browser);
     await browser.url('#/light/table/performance-marks');
     const getMarks = async () => {
       const marks = await browser.execute(() => performance.getEntriesByType('mark') as PerformanceMark[]);
       return marks.filter(m => m.detail?.source === 'awsui');
     };
-    await testFn(page, getMarks);
+    const getElementByPerformanceMark = (id: string) => browser.$(`[data-analytics-performance-mark="${id}"]`);
+
+    await testFn({ page, getMarks, getElementByPerformanceMark });
   });
 }
 
 describe('Table', () => {
   test(
     'Emits a mark only for visible tables',
-    setupTest(async (_, getMarks) => {
+    setupTest(async ({ getMarks, getElementByPerformanceMark }) => {
       const marks = await getMarks();
 
       expect(marks).toHaveLength(2);
@@ -39,12 +47,14 @@ describe('Table', () => {
       });
 
       expect(marks[0].detail.instanceIdentifier).not.toEqual(marks[1].detail.instanceIdentifier);
+
+      expect(await getElementByPerformanceMark(marks[0].detail.instanceIdentifier)).toBeTruthy();
     })
   );
 
   test(
     'Emits a mark when properties change',
-    setupTest(async (page, getMarks) => {
+    setupTest(async ({ page, getMarks, getElementByPerformanceMark }) => {
       await page.click('#loading');
       let marks = await getMarks();
 
@@ -56,6 +66,7 @@ describe('Table', () => {
         loading: false,
         header: 'This is my table',
       });
+      expect(await getElementByPerformanceMark(marks[2].detail.instanceIdentifier)).toBeTruthy();
 
       await page.click('#loading');
 
@@ -68,6 +79,7 @@ describe('Table', () => {
         loading: true,
         header: 'This is my table',
       });
+      expect(await getElementByPerformanceMark(marks[2].detail.instanceIdentifier)).toBeTruthy();
     })
   );
 });
diff --git a/src/table/internal.tsx b/src/table/internal.tsx
@@ -175,7 +175,7 @@ const InternalTable = React.forwardRef(
       toolsHeaderPerformanceMarkRef.current?.querySelector<HTMLElement>(`.${headerStyles['heading-text']}`)
         ?.innerText ?? toolsHeaderPerformanceMarkRef.current?.innerText;
 
-    usePerformanceMarks(
+    const performanceMarkAttributes = usePerformanceMarks(
       'table',
       true,
       tableRefObject,
@@ -442,6 +442,7 @@ const InternalTable = React.forwardRef(
                 getTable={() => tableRefObject.current}
               >
                 <table
+                  {...performanceMarkAttributes}
                   ref={tableRef}
                   className={clsx(
                     styles.table,