From 52a965872585c0ed3093b05506e0b8f1c6aa26bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Dudak?= Date: Mon, 28 Aug 2023 11:28:04 +0200 Subject: [PATCH 1/7] [base-ui] Create hooks contribution guide --- packages/mui-base/CONTRIBUTING.md | 178 ++++++++++++++++++++++++++++++ packages/mui-base/README.md | 4 + 2 files changed, 182 insertions(+) create mode 100644 packages/mui-base/CONTRIBUTING.md diff --git a/packages/mui-base/CONTRIBUTING.md b/packages/mui-base/CONTRIBUTING.md new file mode 100644 index 00000000000000..81043f1a9b3c16 --- /dev/null +++ b/packages/mui-base/CONTRIBUTING.md @@ -0,0 +1,178 @@ +# Contributing + +## Creating a new hook + +### File structure + +When creating a new hook, follow the file structure found in other hooks' directories: + +Taking the imaginary `useAwesomeControl` as an example: + +- πŸ“‚ `useAwesomeControl` + - πŸ“„ `index.ts` - aggregates the public exports from all the other files in the directory + - πŸ“„ `useAwesomeControl.test.tsx` - unit tests + - πŸ“„ `useAwesomeControl.spec.tsx` - (optional) type tests + - πŸ“„ `useAwesomeControl.ts` - the implementation of the hook + - πŸ“„ `useAwesomeControl.types.ts` - type definitions + +### Implementation + +Most Base UI hooks have a similar structure: + +1. [Parameters destructuring](#parameters-destructuring) +2. Hook-specific logic +3. [Event handler factory functions](#event-handler-factory-functions) +4. [Slot props resolvers](#slot-props-resolvers) + +#### Parameters destructuring + +The parameters type must be called `[HookName]Parameters`. +There are docs generation scripts that require this pattern. + +#### Event handler factory functions + +We don't define event handlers directly as functions because they must be able to access and call other handlers provided by developers in slot props resolvers. + +In other words, instead of defining the `handleClick(event: React.MouseEvent) => void` function, we define the `createHandleClick(otherHandlers: EventHandlers) => (event: React.MouseEvent) => void`. The `otherHandlers` parameter contains external handlers provided by developers. + +By convention, we call them `createHandle[EventName]`. + +If we allow a developer to skip running our logic for a given handler, we check the `event.defaultMuiPrevented` field. It's an equivalent of the native `defaultPrevented` that works just for Base UI code: + +```tsx +const createHandleKeyDown = (otherHandlers: EventHandlers) => (event: React.KeyboardEvent & MuiCancellableEvent) => { + // Run the external handler first. + // It possibly can set the defaultMuiPrevented field. + otherHandlers.onKeyDown?.(event); + + // If the field is set, do not execute the usual handler logic. + if (event.defaultMuiPrevented) { + return; + } + + // handler-specific logic... +``` + +#### Slot props resolvers + +These are functions called `get[SlotName]Props` that accept additional (optional) event handlers and return props to be placed on slots of a component. +Many components have just one slot (that we call "root"), but more complex components can have more. + +The order of merging props for the resulting object is important so that users can override the build-in props when necessary: + +1. built-in props +2. external props +3. ref +4. event handlers + +Refs and event handlers can be placed in any order. +They just have to be after external props. + +Example: + +```tsx +const getRootProps = (otherProps: OtherProps = {} as OtherProps): UseAwesomeControlRootSlotProps => { + return { + id, + disabled, + role: 'button' as const, + ...otherProps, // if `id`, `disabled`, or `role` is provided here, they will override the default values set by us. + ref: handleListboxRef, // refs mustn't be overridden, so they come after `...otherProps` + onMouseDown: createHandleMouseDown(otherProps) // here we execute the event handler factory supplying it with external props + }; +}; +``` + +It's common that a hook uses other hooks and combines their `get*Props` with its own. +To handle these cases, we have the `combineHooksSlotProps` utility. +It creates a function that merges two other slot resolver functions: + +```tsx +const createHandleClick = (otherHandlers: EventHandlers) => (event: React.KeyboardEvent) => { + /* ... */ +} + +const { getRootProps as getListRootProps } = useList(/* ... */); +const getOwnRootEventHandlers = (otherHandlers: EventHandlers = {}) => ({ + onClick: createHandleClick(otherHandlers), +}); + +const getRootProps = ( + otherHandlers: TOther = {} as TOther, + ): UseAwesomeControlRootSlotProps => { + const getCombinedRootProps = combineHooksSlotProps(getOwnRootEventHandlers, getListRootProps); + return { + ...getCombinedRootProps(otherHandlers), + role: 'combobox' + } + } + +``` + +#### Ref handling + +When a hook needs to access the DOM node it operates on, it should create a ref and return it in the `get*Props` function. +However, since the user of the hook may already have a ref to the element, we accept the refs in parameters and merge them with our refs using the `useForkRef` function, so the callers of the hook don't have to do it. + +Since hooks can operate on many elements (when dealing with multiple slots), we call refs in input parameters `[slotName]Ref`. + +Example: + +```ts + +interface AwesomeControlHookParameters { + rootRef?: React.Ref; + // ... +} + +const useAwesomeControlHook = (paramters: AwesomeControlHookParameters) { + const { rootRef: externalRef } = parameters; + const innerRef = React.useRef(null); + + const handleRef = useForkRef(externalRef, innerRef); + + return { + // parameters omitted for the sake of brevity + getRootProps: () => { + ref: handleRef + }, + rootRef: handleRef + } +} +``` + +Note that the merged ref (`handleRef`) is not only returned as a part of root props but also as a field of the hook's return object. +This is helpful in situations where the ref needs to be merged with yet another ref. + +### Types + +Defining proper types can tremendously help developers use the hooks. +The following types are required for each hook: + +* [HookName]Parameters - input parameters +* [HookName]ReturnValue - the shape of the object returned by the hook +* [HookName][SlotName]SlotProps - return values of slot props resolvers + +The parameters and return value types are usually straightforward. +The definition of slot props, however, is more complex as it must take into consideration the object passed as an argument to the props resolver function: + +```ts +export interface UseMenuReturnValue { + getListboxProps: ( + otherHandlers?: TOther, + ) => UseMenuListboxSlotProps; + // ... +} + +interface UseMenuListboxSlotEventHandlers { + onBlur: React.FocusEventHandler; + onKeyDown: React.KeyboardEventHandler; +} + +export type UseMenuListboxSlotProps = UseListRootSlotProps< + Omit & UseMenuListboxSlotEventHandlers +> & { + ref: React.RefCallback | null; + role: React.AriaRole; +}; +``` diff --git a/packages/mui-base/README.md b/packages/mui-base/README.md index c21095a94f0490..40e493415bb595 100644 --- a/packages/mui-base/README.md +++ b/packages/mui-base/README.md @@ -19,3 +19,7 @@ yarn add @mui/base [The documentation](https://mui.com/base-ui/getting-started/) + +## Contributing + +See [the contributing guide](./CONTRIBUTING.md) if you wish to implement Base UI components or hooks. From 540e44b4902da8bfa86ba08a310c95de734455ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Dudak?= Date: Mon, 28 Aug 2023 11:44:39 +0200 Subject: [PATCH 2/7] Prettier --- packages/mui-base/CONTRIBUTING.md | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/mui-base/CONTRIBUTING.md b/packages/mui-base/CONTRIBUTING.md index 81043f1a9b3c16..8e79b61b100d7f 100644 --- a/packages/mui-base/CONTRIBUTING.md +++ b/packages/mui-base/CONTRIBUTING.md @@ -71,14 +71,16 @@ They just have to be after external props. Example: ```tsx -const getRootProps = (otherProps: OtherProps = {} as OtherProps): UseAwesomeControlRootSlotProps => { +const getRootProps = ( + otherProps: OtherProps = {} as OtherProps, +): UseAwesomeControlRootSlotProps => { return { id, disabled, role: 'button' as const, ...otherProps, // if `id`, `disabled`, or `role` is provided here, they will override the default values set by us. ref: handleListboxRef, // refs mustn't be overridden, so they come after `...otherProps` - onMouseDown: createHandleMouseDown(otherProps) // here we execute the event handler factory supplying it with external props + onMouseDown: createHandleMouseDown(otherProps), // here we execute the event handler factory supplying it with external props }; }; ``` @@ -128,7 +130,7 @@ interface AwesomeControlHookParameters { const useAwesomeControlHook = (paramters: AwesomeControlHookParameters) { const { rootRef: externalRef } = parameters; const innerRef = React.useRef(null); - + const handleRef = useForkRef(externalRef, innerRef); return { @@ -149,9 +151,9 @@ This is helpful in situations where the ref needs to be merged with yet another Defining proper types can tremendously help developers use the hooks. The following types are required for each hook: -* [HookName]Parameters - input parameters -* [HookName]ReturnValue - the shape of the object returned by the hook -* [HookName][SlotName]SlotProps - return values of slot props resolvers +- [HookName]Parameters - input parameters +- [HookName]ReturnValue - the shape of the object returned by the hook +- [HookName][SlotName]SlotProps - return values of slot props resolvers The parameters and return value types are usually straightforward. The definition of slot props, however, is more complex as it must take into consideration the object passed as an argument to the props resolver function: From eaac590a43b91fd2733fb714e01955482f300e0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Dudak?= Date: Mon, 28 Aug 2023 23:02:22 +0200 Subject: [PATCH 3/7] Rename otherProps to otherHandlers (for now) --- packages/mui-base/CONTRIBUTING.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/mui-base/CONTRIBUTING.md b/packages/mui-base/CONTRIBUTING.md index 8e79b61b100d7f..2bb5667cb0ccec 100644 --- a/packages/mui-base/CONTRIBUTING.md +++ b/packages/mui-base/CONTRIBUTING.md @@ -71,16 +71,16 @@ They just have to be after external props. Example: ```tsx -const getRootProps = ( - otherProps: OtherProps = {} as OtherProps, -): UseAwesomeControlRootSlotProps => { +const getRootProps = ( + otherHandlers: OtherHandlers = {} as OtherHandlers, +): UseAwesomeControlRootSlotProps => { return { id, disabled, role: 'button' as const, - ...otherProps, // if `id`, `disabled`, or `role` is provided here, they will override the default values set by us. - ref: handleListboxRef, // refs mustn't be overridden, so they come after `...otherProps` - onMouseDown: createHandleMouseDown(otherProps), // here we execute the event handler factory supplying it with external props + ...otherHandlers, // if `id`, `disabled`, or `role` is provided here, they will override the default values set by us. + ref: handleListboxRef, // refs mustn't be overridden, so they come after `...otherHandlers` + onMouseDown: createHandleMouseDown(otherHandlers), // here we execute the event handler factory supplying it with external props }; }; ``` From e3a0cae78909168096e0f6c16b1df3d7345652c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Dudak?= Date: Mon, 28 Aug 2023 23:06:47 +0200 Subject: [PATCH 4/7] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Diego Andai Signed-off-by: MichaΕ‚ Dudak --- packages/mui-base/CONTRIBUTING.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/mui-base/CONTRIBUTING.md b/packages/mui-base/CONTRIBUTING.md index 2bb5667cb0ccec..b6969122125d68 100644 --- a/packages/mui-base/CONTRIBUTING.md +++ b/packages/mui-base/CONTRIBUTING.md @@ -55,7 +55,7 @@ const createHandleKeyDown = (otherHandlers: EventHandlers) => (event: React.Keyb #### Slot props resolvers -These are functions called `get[SlotName]Props` that accept additional (optional) event handlers and return props to be placed on slots of a component. +These are functions called `get[SlotName]Props` that accept additional (optional) props and return props to be placed on slots of a component. Many components have just one slot (that we call "root"), but more complex components can have more. The order of merging props for the resulting object is important so that users can override the build-in props when necessary: @@ -127,7 +127,7 @@ interface AwesomeControlHookParameters { // ... } -const useAwesomeControlHook = (paramters: AwesomeControlHookParameters) { +const useAwesomeControlHook = (parameters: AwesomeControlHookParameters) { const { rootRef: externalRef } = parameters; const innerRef = React.useRef(null); From cd942e2c2ebf818179ff575249b51aa89be9c241 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Dudak?= Date: Thu, 31 Aug 2023 12:58:39 +0200 Subject: [PATCH 5/7] Add params destructuring example --- packages/mui-base/CONTRIBUTING.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/mui-base/CONTRIBUTING.md b/packages/mui-base/CONTRIBUTING.md index b6969122125d68..76629539eba122 100644 --- a/packages/mui-base/CONTRIBUTING.md +++ b/packages/mui-base/CONTRIBUTING.md @@ -29,6 +29,17 @@ Most Base UI hooks have a similar structure: The parameters type must be called `[HookName]Parameters`. There are docs generation scripts that require this pattern. +```ts +function useAwesomeControl(parameters: UseAwesomeControlParameters) { + const { + disabled, + readOnly + } = parameters; + + // the rest of the hook's logic... +} +``` + #### Event handler factory functions We don't define event handlers directly as functions because they must be able to access and call other handlers provided by developers in slot props resolvers. From 65712964d3ed1939fc8992b45ddd13273a6c87a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Dudak?= Date: Fri, 1 Sep 2023 08:38:14 +0200 Subject: [PATCH 6/7] Update packages/mui-base/CONTRIBUTING.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sam Sycamore <71297412+samuelsycamore@users.noreply.github.com> Signed-off-by: MichaΕ‚ Dudak --- packages/mui-base/CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mui-base/CONTRIBUTING.md b/packages/mui-base/CONTRIBUTING.md index 76629539eba122..928970b9686adf 100644 --- a/packages/mui-base/CONTRIBUTING.md +++ b/packages/mui-base/CONTRIBUTING.md @@ -53,7 +53,7 @@ If we allow a developer to skip running our logic for a given handler, we check ```tsx const createHandleKeyDown = (otherHandlers: EventHandlers) => (event: React.KeyboardEvent & MuiCancellableEvent) => { // Run the external handler first. - // It possibly can set the defaultMuiPrevented field. + // It can potentially set the defaultMuiPrevented field. otherHandlers.onKeyDown?.(event); // If the field is set, do not execute the usual handler logic. From 4081c72c91848e69375a93ba6d5fa810bd17ed1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Dudak?= Date: Thu, 7 Sep 2023 10:08:34 +0200 Subject: [PATCH 7/7] Prettier --- packages/mui-base/CONTRIBUTING.md | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/mui-base/CONTRIBUTING.md b/packages/mui-base/CONTRIBUTING.md index 928970b9686adf..b1f78a0c01f710 100644 --- a/packages/mui-base/CONTRIBUTING.md +++ b/packages/mui-base/CONTRIBUTING.md @@ -31,11 +31,8 @@ There are docs generation scripts that require this pattern. ```ts function useAwesomeControl(parameters: UseAwesomeControlParameters) { - const { - disabled, - readOnly - } = parameters; - + const { disabled, readOnly } = parameters; + // the rest of the hook's logic... } ```