feat: add runtime warning/errors to components (#2007)

- add utility function for doing the logging
- use in various components (Link, Button, util.s)
- fix violations in EDS identified by these checks
- add additional tests

* chore: address PR comments

src/components/AppNotification/AppNotification.tsx

@@ -84,9 +84,7 @@ export const AppNotification = ({
             onClick={onDismiss}
             rank="tertiary"
             variant={color === 'dark' ? 'inverse' : 'neutral'}
-          >
-            Close
-          </Button>
+          ></Button>
         )}
       </div>
     </div>

src/components/Button/Button.test.tsx

@@ -7,6 +7,18 @@ import * as stories from './Button.stories';
 import type { StoryFile } from '../../util/utility-types';
 
 describe('<Button />', () => {
+  beforeEach(() => {
+    // Add in mocks for the calls that can occur in implementation to suppress logging in tests
+    const consoleMock = jest.spyOn(console, 'error');
+    const consoleWarnMock = jest.spyOn(console, 'warn');
+    consoleMock.mockImplementation();
+    consoleWarnMock.mockImplementation();
+  });
+
+  afterEach(() => {
+    jest.resetAllMocks();
+  });
+
   generateSnapshots(stories as StoryFile);
 
   it('renders the text in the button', () => {
@@ -47,4 +59,32 @@ describe('<Button />', () => {
     const button = screen.getByRole('button');
     expect(button).toHaveFocus();
   });
+
+  describe('emits messages when misused', () => {
+    let consoleErrorMock: jest.SpyInstance, consoleWarnMock: jest.SpyInstance;
+    beforeEach(() => {
+      consoleWarnMock = jest.spyOn(console, 'warn');
+      consoleErrorMock = jest.spyOn(console, 'error');
+      consoleWarnMock.mockImplementation();
+      consoleErrorMock.mockImplementation();
+    });
+
+    it('errors engineers when disable is used', () => {
+      render(<Button disabled>Click</Button>);
+
+      expect(consoleWarnMock).toHaveBeenCalledTimes(0);
+      expect(consoleErrorMock).toHaveBeenCalledTimes(1);
+    });
+
+    it('warns when icon-only Button instances contain children', () => {
+      render(
+        <Button icon="add" iconLayout="icon-only">
+          Click
+        </Button>,
+      );
+
+      expect(consoleWarnMock).toHaveBeenCalledTimes(1);
+      expect(consoleErrorMock).toHaveBeenCalledTimes(0);
+    });
+  });
 });

src/components/Button/Button.tsx

@@ -1,5 +1,6 @@
 import clsx from 'clsx';
 import React, { forwardRef } from 'react';
+import { assertEdsUsage } from '../../util/logging';
 import type { Size } from '../../util/variant-types';
 import Icon, { type IconName } from '../Icon';
 import LoadingIndicator from '../LoadingIndicator';
@@ -132,6 +133,20 @@ export const Button = forwardRef<HTMLButtonElement, ButtonProps>(
       isLoading && styles['button--is-loading'],
     );
 
+    assertEdsUsage(
+      [
+        typeof isDisabled === 'undefined' &&
+          typeof other.disabled !== 'undefined',
+      ],
+      'Use "isDisabled" instead of "disabled" on button instances',
+      'error',
+    );
+
+    assertEdsUsage(
+      [iconLayout === 'icon-only' && typeof children !== 'undefined'],
+      'Specifying content for "children" when using icon-only layout is not required and can be removed.',
+    );
+
     return (
       <Component
         className={componentClassName}

src/components/Link/Link.test.tsx

@@ -7,6 +7,15 @@ import * as stories from './Link.stories';
 import type { StoryFile } from '../../util/utility-types';
 
 describe('<Link />', () => {
+  beforeEach(() => {
+    const consoleMock = jest.spyOn(console, 'warn');
+    consoleMock.mockImplementation();
+  });
+
+  afterEach(() => {
+    jest.resetAllMocks();
+  });
+
   generateSnapshots(stories as StoryFile);
 
   it('renders the text in the link', () => {
@@ -63,4 +72,42 @@ describe('<Link />', () => {
     const link = screen.getByRole('link');
     expect(link).toHaveFocus();
   });
+
+  describe('emits warnings when misused', () => {
+    it('warns when inline links are using emphasis=low', () => {
+      const consoleMock = jest.spyOn(console, 'warn');
+      consoleMock.mockImplementation();
+      render(
+        <Link context="inline" emphasis="low">
+          Click
+        </Link>,
+      );
+
+      expect(consoleMock).toHaveBeenCalledTimes(1);
+    });
+
+    it('warns when inline links have icons specified', () => {
+      const consoleMock = jest.spyOn(console, 'warn');
+      consoleMock.mockImplementation();
+      render(
+        <Link context="inline" icon="open-in-new">
+          Click
+        </Link>,
+      );
+
+      expect(consoleMock).toHaveBeenCalledTimes(1);
+    });
+
+    it('warns when chevron-right is not used in low emphasis mode', () => {
+      const consoleMock = jest.spyOn(console, 'warn');
+      consoleMock.mockImplementation();
+      render(
+        <Link emphasis="high" icon="chevron-right">
+          Click
+        </Link>,
+      );
+
+      expect(consoleMock).toHaveBeenCalledTimes(1);
+    });
+  });
 });

src/components/Link/Link.tsx

@@ -1,5 +1,6 @@
 import clsx from 'clsx';
 import React, { forwardRef } from 'react';
+import { assertEdsUsage } from '../../util/logging';
 import type { Size } from '../../util/variant-types';
 import Icon, { type IconName } from '../Icon';
 
@@ -83,6 +84,21 @@ export const Link = forwardRef<HTMLAnchorElement, LinkProps>(
 
     const iconSize = size && (['xl', 'lg'].includes(size) ? '1.5rem' : '1rem');
 
+    assertEdsUsage(
+      [context === 'inline' && emphasis === 'low'],
+      'Inline links cannot be lowEmphasis',
+    );
+
+    assertEdsUsage(
+      [context === 'inline' && !!icon],
+      'Inline links cannot show icons',
+    );
+
+    assertEdsUsage(
+      [icon === 'chevron-right' && emphasis !== 'low'],
+      'Icon "chevron-right" only allowed when lowEmphasis is used',
+    );
+
     return (
       <Component className={componentClassName} ref={ref} {...other}>
         {children}

src/components/Modal/Modal.tsx

@@ -253,9 +253,7 @@ export const ModalContent = (props: ModalContentProps) => {
             onClick={onClose}
             rank="tertiary"
             variant="neutral"
-          >
-            Close
-          </Button>
+          ></Button>
         )}
         {children}
       </div>

src/components/PageNotification/PageNotification.tsx

@@ -114,9 +114,7 @@ export const PageNotification = ({
           rank="tertiary"
           size="lg"
           variant="neutral"
-        >
-          Close
-        </Button>
+        ></Button>
       )}
     </aside>
   );

src/components/Slider/Slider.stories.tsx

@@ -198,7 +198,7 @@ export const UsingControlButtons: Story = {
       <div className="flex w-full items-center justify-center gap-2">
         <Button
           aria-label="Decrement"
-          disabled={sliderValue === min}
+          isDisabled={sliderValue === min}
           onClick={() => setSliderValue(Math.max(min, sliderValue - 1))}
           rank="secondary"
         >
@@ -218,7 +218,7 @@ export const UsingControlButtons: Story = {
         <Text>{max}</Text>
         <Button
           aria-label="Increment"
-          disabled={sliderValue === max}
+          isDisabled={sliderValue === max}
           onClick={() => setSliderValue(Math.min(max, sliderValue + 1))}
           rank="secondary"
         >

src/components/ToastNotification/ToastNotification.tsx

@@ -95,9 +95,7 @@ export const ToastNotification = ({
           onClick={onDismiss}
           rank="tertiary"
           variant="neutral"
-        >
-          Close
-        </Button>
+        ></Button>
       )}
     </div>
   );

src/util/findLowestTenMultiplier.ts

@@ -1,3 +1,5 @@
+import { assertEdsUsage } from './logging';
+
 /**
  * Returns the lowest multiple of 10 that multiplies with all numbers in a list to make them integers.
  * Useful for floating point math.
@@ -8,12 +10,12 @@
  * @returns {number} Lowest multiple of 10.
  */
 export function findLowestTenMultiplier(numbers: number[]): number {
-  if (
-    process.env.NODE_ENV !== 'production' &&
-    numbers.some((number) => !Number.isFinite(number))
-  ) {
-    throw 'Number should be a real finite number';
-  }
+  assertEdsUsage(
+    [numbers.some((number) => !Number.isFinite(number))],
+    'Number should be a real finite number',
+    'error',
+  );
+
   let multiplier = 1;
   while (
     numbers.some((number) => !Number.isInteger((number * multiplier) % 1))

src/util/logging.ts

@@ -0,0 +1,22 @@
+import identity from 'lodash/identity';
+
+type Check = boolean;
+type LogLevel = 'warn' | 'error';
+
+/**
+ * Logging function used to check whether a usage of EDS is proper and advised. When using, it defaults
+ * to warning, where it will print a message to the console for developers to see. LogLevel supported.
+ *
+ * @param checks set of boolean checks to assert whether the component usages are compatible
+ * @param message Message to print when the component is not being used as advised
+ * @param [loglevel] Severity of the tracked issue
+ */
+export function assertEdsUsage(
+  checks: Check[],
+  message: string,
+  loglevel: LogLevel = 'warn',
+): void {
+  if (process.env.NODE_ENV !== 'production' && [...checks].some(identity)) {
+    console[loglevel](message);
+  }
+}