Document prop merging conventions

contributor-docs/adrs/adr-025-prop-merging.md

@@ -0,0 +1,114 @@
+# Prop merging conventions
+
+📆 Date: 2026-07-28
+
+## Status
+
+| Stage          | State       |
+| -------------- | ----------- |
+| Status         | Proposed ⚠️ |
+| Implementation | Pending ⚠️  |
+
+## Context
+
+Primer components often set props on an element while also accepting those same
+props from consumers. Relying on object spread order alone either replaces
+component behavior and styling or prevents consumers from customizing supported
+props. Inconsistent precedence also makes component APIs difficult to predict.
+
+We need one convention for combining component-authored and consumer-authored
+props, plus an implementation that applies it consistently.
+
+## Decision
+
+Components must intentionally merge any prop that both the component and
+consumer can provide. Pass component props first and consumer props second to the
+`mergeProps` utility:
+
+```tsx
+<button
+  {...mergeProps(
+    {
+      className: classes.Example,
+      onClick: handleClick,
+      style: defaultStyle,
+    },
+    props,
+  )}
+/>
+```
+
+`mergeProps(componentProps, consumerProps)` applies these rules:
+
+- Event handlers are composed in argument order. The component handler runs
+  first. The consumer handler runs only if the component handler does not set
+  `event.defaultPrevented`.
+- `className` values are combined with `clsx`, with the component value first.
+- `style` objects are shallowly merged, with the consumer value taking
+  precedence for duplicate properties.
+- All other duplicate props use the consumer value.
+
+If a component must control an attribute for correct behavior, it must not
+expose that attribute unconditionally. Omit it from the public prop type, or use
+a discriminated union that only exposes it in supported scenarios, rather than
+accepting and silently overriding the consumer value.
+
+The utility is an implementation detail for building Primer React components
+and is not part of the package's public API.
+
+### Scenarios requiring separate handling
+
+The general rules do not cover every form of composition:
+
+- **Refs are not composed.** Use `useMergedRefs` when both the component and
+  consumer need the same element reference.
+- **Styles are not deeply merged.** Nested objects and values such as CSS custom
+  property maps are replaced at the first duplicate property.
+- **Cancellation is based on `defaultPrevented`.** The consumer handler is
+  skipped when the first argument's `defaultPrevented` value is truthy after the
+  component handler runs. Calling `stopPropagation()` alone does not stop the
+  consumer handler because both handlers run for the same React event.
+- **Cancellation is synchronous and one-way.** The consumer cannot prevent the
+  component handler because it runs second, and an asynchronous call to
+  `preventDefault()` occurs too late. APIs that require consumer veto before
+  internal behavior need a separate cancellable callback design.
+- **Errors stop composition.** If the component handler throws, the consumer
+  handler does not run.
+- **`on*` callback props are treated as event handlers.** A callback whose name
+  starts with `on` and whose value is a function is composed with the same
+  cancellation behavior, even if it is not a DOM event handler.
+- **Cancellation only considers the first argument.** All callback arguments
+  are forwarded, but only a `defaultPrevented` value on the first argument can
+  cancel the second callback. Zero-argument callbacks run both handlers.
+- **Ordinary prop precedence includes `undefined`.** An explicitly present
+  consumer prop with an `undefined` value replaces the component value unless
+  the prop has special class name, style, or event-handler behavior.
+
+## Consequences
+
+Component behavior and base styling are preserved while consumers retain
+predictable override points. A shared utility reduces hand-written merging logic
+and makes precedence consistent across components.
+
+The convention requires component authors to distinguish supported overrides
+from attributes that the component must own. Event ordering also means consumer
+handlers cannot cancel component work that has already happened.
+
+## Alternatives
+
+### Use prop spread order for every prop
+
+This is concise, but replaces event handlers, class names, and complete style
+objects instead of composing them.
+
+### Always give component props precedence
+
+This preserves implementation details but makes accepted consumer props
+ineffective and creates a misleading API.
+
+### Let consumers run event handlers first
+
+This would let consumers prevent component behavior, but gives consumers control
+over invariants and accessibility behavior that the component owns. APIs that
+need a consumer veto should model it explicitly instead of changing the default
+ordering for all handlers.

contributor-docs/style.md

@@ -68,6 +68,7 @@ row before the </tbody></table> line.
     - [Utilities](#utilities)
   - [Props](#props)
     - [Prefer applying component rest parameters to the root element rendered by a component](#prefer-applying-component-rest-parameters-to-the-root-element-rendered-by-a-component)
+    - [Merge shared props intentionally](#merge-shared-props-intentionally)
     - [Prefer authoring callback prop types with arguments that can be extended](#prefer-authoring-callback-prop-types-with-arguments-that-can-be-extended)
   - [Hooks](#hooks)
     - [Prefer authoring hooks that accept a `ref` instead of returning a `ref` to apply](#prefer-authoring-hooks-that-accept-a-ref-instead-of-returning-a-ref-to-apply)
@@ -299,6 +300,59 @@ function Example({children, ...rest}: Props) {
 </td></tr>
 </tbody></table>
 
+#### Merge shared props intentionally
+
+When a component and a consumer can both provide a prop, define how those values
+are merged instead of allowing the order of prop spreads to decide accidentally.
+Use the following conventions:
+
+- Event handlers run the component's handler first, then the consumer's handler
+  if the event has not been prevented.
+- Class names are merged with `clsx`.
+- Style objects apply the component's styles first and the consumer's styles
+  second so that the consumer can override them.
+- Other attributes use the consumer's value. If an attribute must be controlled
+  by the component, do not expose it as a prop, or use types that only offer it
+  in the scenarios where the consumer can control it.
+
+Use the `mergeProps` utility with component props first and consumer props
+second:
+
+```tsx
+type Props = React.ComponentPropsWithoutRef<'button'>
+
+function Example(props: Props) {
+  const handleClick = (event: React.MouseEvent<HTMLButtonElement>) => {
+    performInternalAction(event)
+  }
+
+  return (
+    <button
+      {...mergeProps(
+        {
+          'data-component': 'Example',
+          className: classes.Example,
+          onClick: handleClick,
+          style: defaultStyle,
+        },
+        props,
+      )}
+    />
+  )
+}
+```
+
+When a component requires an attribute for correct behavior, remove it from the
+consumer-facing type instead of silently overriding a value the API accepts:
+
+```tsx
+type Props = Omit<React.ComponentPropsWithoutRef<'button'>, 'type'>
+
+function Example(props: Props) {
+  return <button {...props} type="button" />
+}
+```
+
 #### Prefer authoring callback prop types with arguments that can be extended
 
 When authoring callback props, it is helpful to structure the types for these

packages/react/src/utils/__tests__/mergeProps.test.ts

@@ -0,0 +1,84 @@
+import {describe, expect, test, vi} from 'vitest'
+import {mergeProps} from '../mergeProps'
+
+describe('mergeProps', () => {
+  test('combines props and gives the second value precedence', () => {
+    expect(
+      mergeProps(
+        {
+          id: 'component-id',
+          role: 'button',
+        },
+        {
+          id: 'consumer-id',
+          'aria-label': 'Example',
+        },
+      ),
+    ).toEqual({
+      id: 'consumer-id',
+      role: 'button',
+      'aria-label': 'Example',
+    })
+  })
+
+  test('merges class names with clsx', () => {
+    expect(mergeProps({className: 'component'}, {className: ['consumer', {active: true}]})).toEqual({
+      className: 'component consumer active',
+    })
+  })
+
+  test('shallowly merges styles and gives the second value precedence', () => {
+    const componentStyle = {color: 'red', padding: 4}
+    const consumerStyle = {color: 'blue'}
+
+    const merged = mergeProps({style: componentStyle}, {style: consumerStyle})
+
+    expect(merged.style).toEqual({color: 'blue', padding: 4})
+    expect(componentStyle).toEqual({color: 'red', padding: 4})
+    expect(consumerStyle).toEqual({color: 'blue'})
+  })
+
+  test('runs event handlers in order', () => {
+    const calls: string[] = []
+    const event = {defaultPrevented: false}
+    const componentHandler = vi.fn((_event: typeof event, _value: string) => calls.push('component'))
+    const consumerHandler = vi.fn((_event: typeof event, _value: string) => calls.push('consumer'))
+
+    const merged = mergeProps({onClick: componentHandler}, {onClick: consumerHandler})
+    merged.onClick(event, 'value')
+
+    expect(calls).toEqual(['component', 'consumer'])
+    expect(componentHandler).toHaveBeenCalledWith(event, 'value')
+    expect(consumerHandler).toHaveBeenCalledWith(event, 'value')
+  })
+
+  test('does not run the second event handler when the first prevents the event', () => {
+    const event = {
+      defaultPrevented: false,
+      preventDefault() {
+        this.defaultPrevented = true
+      },
+    }
+    const componentHandler = vi.fn((currentEvent: typeof event) => {
+      currentEvent.preventDefault()
+    })
+    const consumerHandler = vi.fn()
+
+    const merged = mergeProps({onClick: componentHandler}, {onClick: consumerHandler})
+    merged.onClick(event)
+
+    expect(componentHandler).toHaveBeenCalledWith(event)
+    expect(consumerHandler).not.toHaveBeenCalled()
+  })
+
+  test('composes zero-argument callbacks', () => {
+    const componentHandler = vi.fn()
+    const consumerHandler = vi.fn()
+
+    const merged = mergeProps({onDismiss: componentHandler}, {onDismiss: consumerHandler})
+    merged.onDismiss()
+
+    expect(componentHandler).toHaveBeenCalledOnce()
+    expect(consumerHandler).toHaveBeenCalledOnce()
+  })
+})

packages/react/src/utils/mergeProps.ts

@@ -0,0 +1,61 @@
+import {clsx} from 'clsx'
+import type {Merge} from './types'
+
+type EventHandler = (...args: unknown[]) => void
+type ClassValue = Parameters<typeof clsx>[number]
+
+function mergeProps<A extends object, B extends object = A>(a: A, b: B): Merge<A, B> {
+  const merged = {...a} as Record<string, unknown>
+
+  for (const [key, value] of Object.entries(b)) {
+    if (key in merged) {
+      const existing = merged[key]
+
+      if (key === 'className') {
+        merged[key] = clsx(existing as ClassValue, value as ClassValue)
+      } else if (isEventHandlerKey(key) && isEventHandler(existing) && isEventHandler(value)) {
+        merged[key] = composeEventHandlers(existing, value)
+      } else if (key === 'style') {
+        merged[key] = mergeStyle(existing, value)
+      } else {
+        merged[key] = value
+      }
+    } else {
+      merged[key] = value
+    }
+  }
+
+  return merged as Merge<A, B>
+}
+
+function composeEventHandlers(a: EventHandler, b: EventHandler) {
+  return (...args: Parameters<EventHandler>) => {
+    a(...args)
+
+    const event = args[0] as {defaultPrevented?: boolean} | undefined
+    if (!event?.defaultPrevented) {
+      b(...args)
+    }
+  }
+}
+
+function isEventHandlerKey(key: string) {
+  return key.startsWith('on')
+}
+
+function isEventHandler(value: unknown): value is EventHandler {
+  return typeof value === 'function'
+}
+
+function mergeStyle(a: unknown, b: unknown) {
+  return {
+    ...(isObject(a) ? a : {}),
+    ...(isObject(b) ? b : {}),
+  }
+}
+
+function isObject(value: unknown): value is Record<string, unknown> {
+  return typeof value === 'object' && value !== null
+}
+
+export {mergeProps}