fix(react-utilities): simplify prop merging in getSlots() (#18861)

* fix typings * Fix getSlots as followed in the RFC * Change files * Update API * Fix tests * Simplify getSlots * Fix react-aria due to changes * Change files * Fix root and test on getSlot * Update react-accordion according to changes on getSlot * Fix lint errors * Adds null case on resolveShorthand
microsoft · Jul 14, 2021 · b602349 · b602349
1 parent a2aad8b
commit b602349
Show file tree

Hide file tree

Showing 15 changed files with 166 additions and 93 deletions.
diff --git a/change/@fluentui-react-accordion-0b135005-6cc1-41a0-a5de-6215eb218e8a.json b/change/@fluentui-react-accordion-0b135005-6cc1-41a0-a5de-6215eb218e8a.json
@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "Fix react-accordion due to changes on slots typings",
+  "packageName": "@fluentui/react-accordion",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/change/@fluentui-react-aria-1f16cbec-6a5c-441c-b31e-aba2558004cf.json b/change/@fluentui-react-aria-1f16cbec-6a5c-441c-b31e-aba2558004cf.json
@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "Fix react-aria due to changes on slots typings",
+  "packageName": "@fluentui/react-aria",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/change/@fluentui-react-utilities-6021096a-5846-499b-a218-b2aeab1e7738.json b/change/@fluentui-react-utilities-6021096a-5846-499b-a218-b2aeab1e7738.json
@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "Fix bugs on getSlots to follow with RFC",
+  "packageName": "@fluentui/react-utilities",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/packages/react-accordion/etc/react-accordion.api.md b/packages/react-accordion/etc/react-accordion.api.md
@@ -4,6 +4,7 @@
 
 ```ts
 
+import { ARIAButtonProps } from '@fluentui/react-aria';
 import { ComponentProps } from '@fluentui/react-utilities';
 import { ComponentState } from '@fluentui/react-utilities';
 import { Context } from '@fluentui/react-context-selector';
@@ -83,7 +84,7 @@ export type AccordionHeaderSize = 'small' | 'medium' | 'large' | 'extra-large';
 
 // @public (undocumented)
 export type AccordionHeaderSlots = {
-    button: React_2.ButtonHTMLAttributes<HTMLElement>;
+    button: ARIAButtonProps;
     expandIcon: AccordionHeaderExpandIconProps;
     icon: React_2.HTMLAttributes<HTMLElement>;
     children: React_2.HTMLAttributes<HTMLElement>;

diff --git a/packages/react-accordion/src/components/AccordionHeader/AccordionHeader.types.ts b/packages/react-accordion/src/components/AccordionHeader/AccordionHeader.types.ts
@@ -1,6 +1,7 @@
 import * as React from 'react';
 import { ComponentProps, ComponentState } from '@fluentui/react-utilities';
 import { AccordionHeaderExpandIconProps } from './AccordionHeaderExpandIcon';
+import { ARIAButtonProps } from '@fluentui/react-aria';
 
 export type AccordionHeaderSize = 'small' | 'medium' | 'large' | 'extra-large';
 export type AccordionHeaderExpandIconPosition = 'start' | 'end';
@@ -16,7 +17,7 @@ export type AccordionHeaderSlots = {
   /**
    * The component to be used as button in heading
    */
-  button: React.ButtonHTMLAttributes<HTMLElement>;
+  button: ARIAButtonProps;
   /**
    * Expand icon slot rendered before (or after) children content in heading
    */

diff --git a/packages/react-accordion/src/components/AccordionHeader/useAccordionHeader.ts b/packages/react-accordion/src/components/AccordionHeader/useAccordionHeader.ts
@@ -67,7 +67,7 @@ export const useAccordionHeader = (props: AccordionHeaderProps, ref: React.Ref<H
     }),
     button: {
       ...buttonShorthand,
-      onClick: useEventCallback((ev: React.MouseEvent<HTMLElement>) => {
+      onClick: useEventCallback(ev => {
         buttonShorthand.onClick?.(ev);
         if (!ev.defaultPrevented) {
           onAccordionHeaderClick(ev);

diff --git a/packages/react-aria/etc/react-aria.api.md b/packages/react-aria/etc/react-aria.api.md
@@ -8,8 +8,26 @@ import { ObjectShorthandProps } from '@fluentui/react-utilities';
 import * as React_2 from 'react';
 import { ShorthandProps } from '@fluentui/react-utilities';
 
+// @public (undocumented)
+export type ARIAButtonAsAnchorProps = React_2.AnchorHTMLAttributes<HTMLAnchorElement> & {
+    as: 'a';
+};
+
+// @public (undocumented)
+export type ARIAButtonAsButtonProps = React_2.ButtonHTMLAttributes<HTMLButtonElement> & {
+    as?: 'button';
+};
+
+// @public (undocumented)
+export type ARIAButtonAsElementProps = React_2.HTMLAttributes<HTMLElement> & {
+    as: 'div' | 'span';
+};
+
+// @public (undocumented)
+export type ARIAButtonProps = ARIAButtonAsButtonProps | ARIAButtonAsElementProps | ARIAButtonAsAnchorProps;
+
 // @public
-export function useARIAButton<T extends React_2.ButtonHTMLAttributes<HTMLElement>>(value: ShorthandProps<T>, defaultProps?: T): ObjectShorthandProps<T>;
+export function useARIAButton(value: ShorthandProps<ARIAButtonProps>, defaultProps?: ARIAButtonProps): ObjectShorthandProps<ARIAButtonProps>;
 
 
 // (No @packageDocumentation comment for this package)

diff --git a/packages/react-aria/src/hooks/useARIAButton.stories.tsx b/packages/react-aria/src/hooks/useARIAButton.stories.tsx
@@ -1,15 +1,30 @@
-import { getSlots } from '@fluentui/react-utilities';
+import { ComponentState, getSlots } from '@fluentui/react-utilities';
 import * as React from 'react';
-import { useARIAButton } from './useARIAButton';
+import { ARIAButtonAsElementProps, ARIAButtonProps, useARIAButton } from './useARIAButton';
+
+type Slots = {
+  button: ARIAButtonProps;
+};
+
+interface State extends ComponentState<Slots> {}
 
 interface DefaultArgs {
   onClick: (ev: React.MouseEvent) => void;
 }
 
 export const Default = (args: DefaultArgs) => {
-  const props = useARIAButton({ as: 'button', onClick: args.onClick });
-  const { slots, slotProps } = getSlots(props, []);
-  return <slots.root {...slotProps.root}>this is a button</slots.root>;
+  const state: State = {
+    button: {
+      ...useARIAButton({ as: 'button', onClick: args.onClick }),
+      children: React.Fragment,
+    },
+  };
+  const { slots, slotProps } = getSlots<Slots>(state, ['button']);
+  return (
+    <slots.root {...slotProps.root}>
+      <slots.button {...(slotProps.button as ARIAButtonAsElementProps)}>this is a button</slots.button>
+    </slots.root>
+  );
 };
 
 export const Anchor = (args: DefaultArgs) => {

diff --git a/packages/react-aria/src/hooks/useARIAButton.test.tsx b/packages/react-aria/src/hooks/useARIAButton.test.tsx
@@ -1,15 +1,15 @@
 import * as React from 'react';
-import { useARIAButton } from './useARIAButton';
+import { ARIAButtonProps, useARIAButton } from './useARIAButton';
 import { EnterKey, SpacebarKey } from '@fluentui/keyboard-key';
 import { renderHook } from '@testing-library/react-hooks';
 import { fireEvent, screen, render } from '@testing-library/react';
-import { getSlotsCompat, ObjectShorthandProps } from '@fluentui/react-utilities';
+import { getSlots, ObjectShorthandProps } from '@fluentui/react-utilities';
 
 describe('useARIAButton', () => {
   it('should return by default shorthand props for a button', () => {
-    const shorthand: ObjectShorthandProps<React.ButtonHTMLAttributes<HTMLElement>> = {};
+    const shorthand: ObjectShorthandProps<ARIAButtonProps> = {};
     renderHook(() => useARIAButton(shorthand));
-    expect(shorthand.as).toBe('button');
+    expect(shorthand.as).toBe(undefined);
     expect(shorthand.disabled).toBeUndefined();
     expect(shorthand['aria-disabled']).toBeUndefined();
     expect(shorthand.role).toBeUndefined();
@@ -19,10 +19,9 @@ describe('useARIAButton', () => {
     expect(shorthand.onKeyUp).toBeUndefined();
   });
   it('should return handlers for anchor when anchor element is declared', () => {
-    const shorthand: ObjectShorthandProps<React.ButtonHTMLAttributes<HTMLElement>> = { as: 'a' };
+    const shorthand: ObjectShorthandProps<ARIAButtonProps> = { as: 'a' };
     renderHook(() => useARIAButton(shorthand));
     expect(shorthand.as).toBe('a');
-    expect(shorthand.disabled).toBeUndefined();
     expect(shorthand['aria-disabled']).toBe(false);
     expect(shorthand.role).toBe('button');
     expect(shorthand.onClick).toBeInstanceOf(Function);
@@ -31,11 +30,10 @@ describe('useARIAButton', () => {
     expect(shorthand.onKeyUp).toBeInstanceOf(Function);
   });
   it('should return handlers when shorthand props declares another semantic element', () => {
-    const shorthand: ObjectShorthandProps<React.ButtonHTMLAttributes<HTMLElement>> = { as: 'div' };
+    const shorthand: ObjectShorthandProps<ARIAButtonProps> = { as: 'div' };
     renderHook(() => useARIAButton(shorthand));
     expect(shorthand.as).toBe('div');
     expect(shorthand.role).toBe('button');
-    expect(shorthand.disabled).toBeUndefined();
     expect(shorthand['aria-disabled']).toBe(false);
     expect(shorthand.tabIndex).toBe(0);
     expect(shorthand.onClick).toBeInstanceOf(Function);
@@ -46,7 +44,7 @@ describe('useARIAButton', () => {
   it('should emit click events on Click', () => {
     const handleClick = jest.fn();
     const { result } = renderHook(() => useARIAButton({ as: 'div', onClick: handleClick }));
-    const { slots, slotProps } = getSlotsCompat(result.current, []);
+    const { slots, slotProps } = getSlots(result.current, []);
     render(<slots.root data-testid="div" {...slotProps.root} />);
     fireEvent.click(screen.getByTestId('div'));
     expect(handleClick).toHaveBeenCalledTimes(1);
@@ -55,7 +53,7 @@ describe('useARIAButton', () => {
   it('should emit click events on SpaceBar', () => {
     const handleClick = jest.fn();
     const { result } = renderHook(() => useARIAButton({ as: 'div', onClick: handleClick }));
-    const { slots, slotProps } = getSlotsCompat(result.current, []);
+    const { slots, slotProps } = getSlots(result.current, []);
     render(<slots.root data-testid="div" {...slotProps.root} />);
     fireEvent.keyUp(screen.getByTestId('div'), { keyCode: SpacebarKey });
     expect(handleClick).toHaveBeenCalledTimes(1);
@@ -64,16 +62,16 @@ describe('useARIAButton', () => {
   it('should emit click events on Enter', () => {
     const handleClick = jest.fn();
     const { result } = renderHook(() => useARIAButton({ as: 'div', onClick: handleClick }));
-    const { slots, slotProps } = getSlotsCompat(result.current, []);
+    const { slots, slotProps } = getSlots(result.current, []);
     render(<slots.root data-testid="div" {...slotProps.root} />);
     fireEvent.keyDown(screen.getByTestId('div'), { keyCode: EnterKey });
     expect(handleClick).toHaveBeenCalledTimes(1);
   });
 
   it('should prevent default and stop propagation on disabled while clicking', () => {
     const handleClick = jest.fn();
-    const { result } = renderHook(() => useARIAButton({ as: 'div', disabled: true, onClick: handleClick }));
-    const { slots, slotProps } = getSlotsCompat(result.current, []);
+    const { result } = renderHook(() => useARIAButton({ as: 'div', 'aria-disabled': true, onClick: handleClick }));
+    const { slots, slotProps } = getSlots(result.current, []);
     render(
       <div onClick={handleClick}>
         <slots.root data-testid="div" {...slotProps.root} />
@@ -85,8 +83,8 @@ describe('useARIAButton', () => {
 
   it('should prevent default and stop propagation on disabled while pressing SpaceBar', () => {
     const handleClick = jest.fn();
-    const { result } = renderHook(() => useARIAButton({ as: 'div', disabled: true, onClick: handleClick }));
-    const { slots, slotProps } = getSlotsCompat(result.current, []);
+    const { result } = renderHook(() => useARIAButton({ as: 'div', 'aria-disabled': true, onClick: handleClick }));
+    const { slots, slotProps } = getSlots(result.current, []);
     render(
       <div onClick={handleClick}>
         <slots.root data-testid="div" {...slotProps.root} />
@@ -98,8 +96,8 @@ describe('useARIAButton', () => {
 
   it('should prevent default and stop propagation on disabled while pressing Enter', () => {
     const handleClick = jest.fn();
-    const { result } = renderHook(() => useARIAButton({ as: 'div', disabled: true, onClick: handleClick }));
-    const { slots, slotProps } = getSlotsCompat(result.current, []);
+    const { result } = renderHook(() => useARIAButton({ as: 'div', 'aria-disabled': true, onClick: handleClick }));
+    const { slots, slotProps } = getSlots(result.current, []);
     render(
       <div onClick={handleClick}>
         <slots.root data-testid="div" {...slotProps.root} />

diff --git a/packages/react-aria/src/hooks/useARIAButton.ts b/packages/react-aria/src/hooks/useARIAButton.ts
@@ -9,21 +9,28 @@ function mergeARIADisabled(disabled?: boolean | 'false' | 'true'): boolean {
   return disabled ?? false;
 }
 
+export type ARIAButtonAsButtonProps = React.ButtonHTMLAttributes<HTMLButtonElement> & { as?: 'button' };
+export type ARIAButtonAsElementProps = React.HTMLAttributes<HTMLElement> & { as: 'div' | 'span' };
+export type ARIAButtonAsAnchorProps = React.AnchorHTMLAttributes<HTMLAnchorElement> & { as: 'a' };
+
+export type ARIAButtonProps = ARIAButtonAsButtonProps | ARIAButtonAsElementProps | ARIAButtonAsAnchorProps;
+
 /**
  * button keyboard handling, role, disabled and tabIndex implementation that ensures ARIA spec
  * for multiple scenarios of shorthand properties. Ensuring 1st rule of ARIA for cases
  * where no attribute addition is required
  */
-export function useARIAButton<T extends React.ButtonHTMLAttributes<HTMLElement>>(
-  value: ShorthandProps<T>,
-  defaultProps?: T,
-): ObjectShorthandProps<T> {
+export function useARIAButton(
+  value: ShorthandProps<ARIAButtonProps>,
+  defaultProps?: ARIAButtonProps,
+): ObjectShorthandProps<ARIAButtonProps> {
   const shorthand = resolveShorthand(value, defaultProps);
 
-  const { onClick, onKeyDown, onKeyUp, disabled: defaultDisabled, ['aria-disabled']: ariaDisabled } = shorthand;
-  const disabled = mergeARIADisabled(defaultDisabled ?? ariaDisabled);
+  const { onClick, onKeyDown, onKeyUp, ['aria-disabled']: ariaDisabled } = shorthand;
+
+  const disabled = mergeARIADisabled((shorthand.as === 'button' ? shorthand.disabled : undefined) ?? ariaDisabled);
 
-  const onClickHandler = useEventCallback((ev: React.MouseEvent<HTMLElement>) => {
+  const onClickHandler: ARIAButtonProps['onClick'] = useEventCallback(ev => {
     if (disabled) {
       ev.preventDefault();
       ev.stopPropagation();
@@ -34,7 +41,7 @@ export function useARIAButton<T extends React.ButtonHTMLAttributes<HTMLElement>>
     }
   });
 
-  const onKeyDownHandler = useEventCallback((ev: React.KeyboardEvent<HTMLElement>) => {
+  const onKeyDownHandler: ARIAButtonProps['onKeyDown'] = useEventCallback(ev => {
     const code = getCode(ev);
     if (typeof onKeyDown === 'function') {
       onKeyDown(ev);
@@ -58,7 +65,7 @@ export function useARIAButton<T extends React.ButtonHTMLAttributes<HTMLElement>>
     }
   });
 
-  const onKeyupHandler = useEventCallback((ev: React.KeyboardEvent<HTMLElement>) => {
+  const onKeyupHandler: ARIAButtonProps['onKeyUp'] = useEventCallback(ev => {
     const code = getCode(ev);
     if (typeof onKeyUp === 'function') {
       onKeyUp(ev);
@@ -77,9 +84,8 @@ export function useARIAButton<T extends React.ButtonHTMLAttributes<HTMLElement>>
     }
   });
 
-  if (!shorthand.hasOwnProperty('as') || shorthand.as === 'button') {
-    shorthand.as = 'button';
-    return shorthand; // there's nothing to be done if as prop === 'button'
+  if (shorthand.as === 'button' || shorthand.as === undefined) {
+    return shorthand; // there's nothing to be done if as prop === 'button' or undefined
   }
 
   if (!shorthand.hasOwnProperty('role')) {

diff --git a/packages/react-utilities/etc/react-utilities.api.md b/packages/react-utilities/etc/react-utilities.api.md
@@ -122,10 +122,10 @@ export function getNativeProps<T extends Record<string, any>>(props: Record<stri
 // @public
 export function getSlots<SlotProps extends SlotPropsRecord = {}>(state: ComponentState<any>, slotNames?: string[]): {
     readonly slots: { [K in keyof SlotProps]: React_2.ElementType<SlotProps[K]>; } & {
-        root: React_2.ElementType;
+        readonly root: React_2.ElementType<any>;
     };
-    readonly slotProps: SlotProps & {
-        root: any;
+    readonly slotProps: { [Key in keyof SlotProps]: UnionToIntersection<SlotProps[Key]>; } & {
+        readonly root: any;
     };
 };
 
@@ -177,7 +177,7 @@ export const nullRender: () => null;
 // @public (undocumented)
 export type ObjectShorthandProps<Props extends {
     children?: React_2.ReactNode;
-} = {}> = Props & Pick<ComponentProps, 'as'> & {
+} = {}> = Props & {
     children?: Props['children'] | ShorthandRenderFunction<Props>;
 };
 
@@ -380,6 +380,7 @@ export const videoProperties: Record<string, number>;
 
 // Warnings were encountered during analysis:
 //
+// lib/compose/getSlots.d.ts:27:5 - (ae-forgotten-export) The symbol "UnionToIntersection" needs to be exported by the entry point index.d.ts
 // lib/descendants/descendants.d.ts:64:5 - (ae-forgotten-export) The symbol "SomeElement" needs to be exported by the entry point index.d.ts
 
 // (No @packageDocumentation comment for this package)