Skip to content

Commit 6c82171

Browse files
wangdicoderwangdicoder
committed
fix(react): normalize ref exposure policy
1 parent d4fcf7f commit 6c82171

15 files changed

Lines changed: 290 additions & 206 deletions

File tree

‎packages/react/REFS.md‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Ref Conventions
2+
3+
This package treats `ref` as an explicit component capability, not an accidental side effect of inheriting DOM props.
4+
5+
## Rules
6+
7+
- Use `React.ComponentPropsWithoutRef<'tag'>` for DOM-derived prop interfaces by default.
8+
- Only expose `ref` when the component intentionally supports DOM access through `React.forwardRef`.
9+
- Do not rely on `React.ComponentProps<'tag'>` to leak `ref` into props interfaces.
10+
- If a component is primarily compositional or layout-only, do not expose `ref` unless there is a concrete DOM integration need.
11+
12+
## Components That Should Expose `ref`
13+
14+
- Interactive controls and focus targets such as `Button`, `Input`, `NativeSelect`, `Link`, `Switch`, `Checkbox`, `Radio`.
15+
- Components frequently used for measurement, scrolling, positioning, or animation integration such as `Tree`, `Slider`, `Grid`, `Space`, `Flex`, `Layout`, `Anchor`, `Waterfall`, `Transfer`, `Steps`, and typography primitives.
16+
17+
## Components That Usually Should Not Expose `ref`
18+
19+
- Grouping or compositional wrappers such as `Button.Group`, `Checkbox.Group`, `Radio.Group`, `Input.Group`, `Input.Addon`, `Form.Item`, `Descriptions`, `Descriptions.Item`.
20+
21+
## Practical Guidance
22+
23+
- If consumers need the outer wrapper and an inner native control, expose both intentionally, for example `Checkbox` plus `checkboxRef`, or `Radio` plus `radioRef`.
24+
- If a component renders different DOM elements by state, make the forwarded ref type reflect that reality instead of pretending it always points to one tag.
25+
- When adding a new forwarded ref, add a focused test that proves the ref resolves to the expected DOM node.

‎packages/react/src/anchor/__tests__/anchor.test.tsx‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,16 @@ describe('<Anchor />', () => {
3232
expect(getByText('Link 1')).toBeInTheDocument();
3333
expect(getByText('Link 2')).toBeInTheDocument();
3434
});
35+
36+
it('should forward ref to root list element', () => {
37+
const ref = React.createRef<HTMLUListElement>();
38+
39+
render(
40+
<Anchor ref={ref}>
41+
<Anchor.Link href="#section1" title="Section 1" />
42+
</Anchor>
43+
);
44+
45+
expect(ref.current).toBeInstanceOf(HTMLUListElement);
46+
});
3547
});

‎packages/react/src/anchor/anchor.tsx‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,20 @@ import { AnchorLinkProps, AnchorProps } from './types';
77
import { AnchorContext } from './anchor-context';
88
import Sticky from '../sticky';
99

10-
const Anchor = (props: AnchorProps): JSX.Element => {
10+
function assignRef<T>(ref: React.Ref<T> | undefined, value: T | null): void {
11+
if (!ref) {
12+
return;
13+
}
14+
15+
if (typeof ref === 'function') {
16+
ref(value);
17+
return;
18+
}
19+
20+
(ref as React.MutableRefObject<T | null>).current = value;
21+
}
22+
23+
const Anchor = React.forwardRef<HTMLUListElement, AnchorProps>((props, ref): JSX.Element => {
1124
const {
1225
affix = false,
1326
offsetTop = 0,
@@ -20,6 +33,7 @@ const Anchor = (props: AnchorProps): JSX.Element => {
2033
style,
2134
children,
2235
prefixCls: customisedCls,
36+
...otherProps
2337
} = props;
2438
const configContext = useContext(ConfigContext);
2539
const prefixCls = getPrefixCls('anchor', configContext.prefixCls, customisedCls);
@@ -37,6 +51,14 @@ const Anchor = (props: AnchorProps): JSX.Element => {
3751
linksRef.current.delete(href);
3852
}, []);
3953

54+
const setAnchorNode = useCallback(
55+
(node: HTMLUListElement | null) => {
56+
anchorRef.current = node;
57+
assignRef(ref, node);
58+
},
59+
[ref]
60+
);
61+
4062
const updateInk = useCallback(() => {
4163
const anchorEl = anchorRef.current;
4264
if (!anchorEl) return;
@@ -200,7 +222,7 @@ const Anchor = (props: AnchorProps): JSX.Element => {
200222
);
201223

202224
const anchorContent = (
203-
<ul className={cls} style={style} ref={anchorRef}>
225+
<ul {...otherProps} className={cls} style={style} ref={setAnchorNode}>
204226
<div className={`${prefixCls}__ink`}>
205227
<div className={`${prefixCls}__ink-ball`} ref={inkBallRef} />
206228
</div>
@@ -228,7 +250,7 @@ const Anchor = (props: AnchorProps): JSX.Element => {
228250
)}
229251
</AnchorContext.Provider>
230252
);
231-
};
253+
});
232254

233255
Anchor.displayName = 'Anchor';
234256

‎packages/react/src/anchor/types.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import React from 'react';
22
import { BaseProps } from '../_utils/props';
33

4-
export interface AnchorProps extends BaseProps {
4+
export interface AnchorProps
5+
extends BaseProps,
6+
Omit<React.ComponentPropsWithoutRef<'ul'>, 'children' | 'onChange' | 'onClick'> {
57
affix?: boolean;
68
type?: 'dot' | 'line';
79
offsetBottom?: number;

‎packages/react/src/button/button-group.tsx‎

Lines changed: 62 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -8,76 +8,74 @@ import { BUTTON_MARK } from './button';
88

99
const hasOwnProp = <T extends object>(props: T, key: keyof T): boolean => props[key] !== undefined;
1010

11-
const ButtonGroup = React.forwardRef<HTMLDivElement, ButtonGroupProps>(
12-
(props: ButtonGroupProps, ref) => {
13-
const {
14-
size = 'md',
15-
variant = 'solid',
16-
color = 'default',
17-
disabled = false,
18-
round = false,
19-
shape,
20-
inheritMode = 'fill',
21-
prefixCls: customisedCls,
22-
className,
23-
children,
24-
...otherProps
25-
} = props;
26-
const configContext = useContext(ConfigContext);
27-
const prefixCls = getPrefixCls('btn-group', configContext.prefixCls, customisedCls);
28-
const btnSize = props.size || configContext.componentSize || size;
29-
const resolvedShape = shape || (round ? 'round' : 'default');
30-
const cls = classNames(
31-
prefixCls,
32-
{
33-
[`${prefixCls}_round`]: resolvedShape === 'round',
34-
[`${prefixCls}_variant-${variant}`]: variant,
35-
[`${prefixCls}_color-${color}`]: color,
36-
[`${prefixCls}_${btnSize}`]: btnSize,
37-
},
38-
className
39-
);
40-
return (
41-
<div {...otherProps} className={cls} ref={ref}>
42-
{React.Children.map(children, (child) => {
43-
if (!React.isValidElement<ButtonProps>(child)) {
44-
return child;
45-
}
11+
const ButtonGroup = (props: ButtonGroupProps): React.ReactElement => {
12+
const {
13+
size = 'md',
14+
variant = 'solid',
15+
color = 'default',
16+
disabled = false,
17+
round = false,
18+
shape,
19+
inheritMode = 'fill',
20+
prefixCls: customisedCls,
21+
className,
22+
children,
23+
...otherProps
24+
} = props;
25+
const configContext = useContext(ConfigContext);
26+
const prefixCls = getPrefixCls('btn-group', configContext.prefixCls, customisedCls);
27+
const btnSize = props.size || configContext.componentSize || size;
28+
const resolvedShape = shape || (round ? 'round' : 'default');
29+
const cls = classNames(
30+
prefixCls,
31+
{
32+
[`${prefixCls}_round`]: resolvedShape === 'round',
33+
[`${prefixCls}_variant-${variant}`]: variant,
34+
[`${prefixCls}_color-${color}`]: color,
35+
[`${prefixCls}_${btnSize}`]: btnSize,
36+
},
37+
className
38+
);
39+
return (
40+
<div {...otherProps} className={cls}>
41+
{React.Children.map(children, (child) => {
42+
if (!React.isValidElement<ButtonProps>(child)) {
43+
return child;
44+
}
4645

47-
const childType = child.type as Record<PropertyKey, unknown>;
48-
if (!childType[BUTTON_MARK]) {
49-
return child;
50-
}
46+
const childType = child.type as unknown as Record<PropertyKey, unknown>;
47+
if (!childType[BUTTON_MARK]) {
48+
return child;
49+
}
5150

52-
if (inheritMode === 'none') {
53-
return child;
54-
}
51+
if (inheritMode === 'none') {
52+
return child;
53+
}
5554

56-
const childProps: Partial<ButtonProps> = {};
57-
const applyProp = <K extends keyof ButtonProps>(key: K, value: ButtonProps[K]): void => {
58-
if (inheritMode === 'override' || !hasOwnProp(child.props, key)) {
59-
childProps[key] = value;
60-
}
61-
};
55+
const childProps: Partial<ButtonProps> = {};
56+
const applyProp = <K extends keyof ButtonProps>(key: K, value: ButtonProps[K]): void => {
57+
if (inheritMode === 'override' || !hasOwnProp(child.props, key)) {
58+
childProps[key] = value;
59+
}
60+
};
6261

63-
applyProp('variant', variant as ButtonVariant);
64-
applyProp('color', color as ButtonColor);
65-
applyProp('size', btnSize as SizeType);
66-
applyProp('disabled', disabled);
62+
applyProp('variant', variant as ButtonVariant);
63+
applyProp('color', color as ButtonColor);
64+
applyProp('size', btnSize as SizeType);
65+
applyProp('disabled', disabled);
6766

68-
if (
69-
inheritMode === 'override' ||
70-
(!hasOwnProp(child.props, 'shape') && !hasOwnProp(child.props, 'round'))
71-
) {
72-
childProps.shape = resolvedShape as ButtonShape;
73-
}
67+
if (
68+
inheritMode === 'override' ||
69+
(!hasOwnProp(child.props, 'shape') && !hasOwnProp(child.props, 'round'))
70+
) {
71+
childProps.shape = resolvedShape as ButtonShape;
72+
}
7473

75-
return React.cloneElement(child, childProps);
76-
})}
77-
</div>
78-
);
79-
}
80-
);
74+
return React.cloneElement(child, childProps);
75+
})}
76+
</div>
77+
);
78+
};
8179

8280
ButtonGroup.displayName = 'ButtonGroup';
8381

‎packages/react/src/button/button.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { getPrefixCls } from '../_utils/general';
55
import { ButtonProps } from './types';
66

77
export const BUTTON_MARK = Symbol('tiny-design.button');
8+
const isProduction = (globalThis as { process?: { env?: { NODE_ENV?: string } } }).process?.env?.NODE_ENV === 'production';
89

910
const Button = React.forwardRef<HTMLButtonElement, ButtonProps>((props: ButtonProps, ref) => {
1011
const {
@@ -37,7 +38,7 @@ const Button = React.forwardRef<HTMLButtonElement, ButtonProps>((props: ButtonPr
3738
const accessibleName =
3839
otherProps['aria-label'] || otherProps['aria-labelledby'] || otherProps.title;
3940

40-
if (process.env.NODE_ENV !== 'production' && isIconOnly && !accessibleName) {
41+
if (!isProduction && isIconOnly && !accessibleName) {
4142
// Icon-only buttons need an accessible name.
4243
console.warn(
4344
'Button with icon only should provide `aria-label`, `aria-labelledby`, or `title`.'

‎packages/react/src/checkbox/checkbox-group.tsx‎

Lines changed: 42 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -5,53 +5,51 @@ import { getPrefixCls } from '../_utils/general';
55
import { CheckboxGroupContext } from './checkbox-group-context';
66
import { CheckboxGroupProps } from './types';
77

8-
const CheckboxGroup = React.forwardRef<HTMLDivElement, CheckboxGroupProps>(
9-
(props: CheckboxGroupProps, ref): React.ReactElement => {
10-
const {
11-
defaultValue = [],
12-
prefixCls: customisedCls,
13-
onChange,
14-
disabled,
15-
className,
16-
children,
17-
...otherProps
18-
} = props;
19-
const configContext = useContext(ConfigContext);
20-
const prefixCls = getPrefixCls('checkbox-group', configContext.prefixCls, customisedCls);
21-
const cls = classNames(prefixCls, className);
22-
const [value, setValue] = useState<string[]>(
23-
'value' in props ? (props.value as string[]) : defaultValue
24-
);
8+
const CheckboxGroup = (props: CheckboxGroupProps): React.ReactElement => {
9+
const {
10+
defaultValue = [],
11+
prefixCls: customisedCls,
12+
onChange,
13+
disabled,
14+
className,
15+
children,
16+
...otherProps
17+
} = props;
18+
const configContext = useContext(ConfigContext);
19+
const prefixCls = getPrefixCls('checkbox-group', configContext.prefixCls, customisedCls);
20+
const cls = classNames(prefixCls, className);
21+
const [value, setValue] = useState<string[]>(
22+
'value' in props ? (props.value as string[]) : defaultValue
23+
);
2524

26-
const itemOnChange = (e: React.ChangeEvent<HTMLInputElement>): void => {
27-
if (!disabled) {
28-
const name = e.currentTarget.name;
29-
const newValue = value.includes(name)
30-
? value.filter((v) => v !== name)
31-
: [...value, name];
32-
!('value' in props) && setValue(newValue);
33-
onChange && onChange(newValue);
34-
}
35-
};
25+
const itemOnChange = (e: React.ChangeEvent<HTMLInputElement>): void => {
26+
if (!disabled) {
27+
const name = e.currentTarget.name;
28+
const newValue = value.includes(name)
29+
? value.filter((v) => v !== name)
30+
: [...value, name];
31+
!('value' in props) && setValue(newValue);
32+
onChange && onChange(newValue);
33+
}
34+
};
3635

37-
useEffect(() => {
38-
'value' in props && setValue([...(props.value as string[])]);
39-
}, [props]);
36+
useEffect(() => {
37+
'value' in props && setValue([...(props.value as string[])]);
38+
}, [props]);
4039

41-
return (
42-
<CheckboxGroupContext.Provider
43-
value={{
44-
value,
45-
disabled,
46-
onChange: itemOnChange,
47-
}}>
48-
<div {...otherProps} ref={ref} role="group" className={cls}>
49-
{children}
50-
</div>
51-
</CheckboxGroupContext.Provider>
52-
);
53-
}
54-
);
40+
return (
41+
<CheckboxGroupContext.Provider
42+
value={{
43+
value,
44+
disabled,
45+
onChange: itemOnChange,
46+
}}>
47+
<div {...otherProps} role="group" className={cls}>
48+
{children}
49+
</div>
50+
</CheckboxGroupContext.Provider>
51+
);
52+
};
5553

5654
CheckboxGroup.displayName = 'CheckboxGroup';
5755

‎packages/react/src/collapse/collapse-panel.tsx‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,7 @@ import React, { useEffect, useId, useState } from 'react';
22
import classNames from 'classnames';
33
import { ArrowDown } from '../_utils/components';
44
import CollapseTransition from '../collapse-transition';
5-
import {
6-
CollapseCollapsible,
7-
CollapseExpandIconRender,
8-
CollapseItem,
9-
CollapseRenderState,
10-
} from './types';
5+
import { CollapseCollapsible, CollapseExpandIconRender, CollapseItem, CollapseRenderState } from './types';
116

127
type CollapsePanelProps = {
138
prefixCls: string;
@@ -28,10 +23,10 @@ type CollapsePanelProps = {
2823
onToggle: (key: string, event: React.MouseEvent) => void;
2924
};
3025

31-
const renderContent = <T extends React.ReactNode | ((args: CollapseRenderState) => React.ReactNode)>(
32-
content: T,
26+
const renderContent = (
27+
content: React.ReactNode | ((args: CollapseRenderState) => React.ReactNode),
3328
state: CollapseRenderState
34-
) => {
29+
): React.ReactNode => {
3530
return typeof content === 'function' ? content(state) : content;
3631
};
3732

0 commit comments

Comments
 (0)