Skip to content

Commit c877f5c

Browse files
yaminyassinYamin Yassin
andauthored
Type-safe props, events, and refs for react-strict-dom (#504)
* Type event-handler props instead of $FlowFixMe The event props on the Strict* prop types were all $FlowFixMe, so handlers got no checking and authors got no autocomplete. Add StrictReactDOMEvents with the event shapes the native factories actually build (change, input, key, click, image load/error) and a StrictOpaqueEventHandler for the pass-through handlers whose runtime shape is platform-specific. Wire those through StrictReactDOMProps and the button/image/input/select/ textarea prop types, and re-export the payload types from the native and web entrypoints so consumers can annotate their own handlers. * Clear out the $FlowFixMe in the native renderer Type the props the native modules build and drop the suppressions that were hiding the gaps. The prop mutation that the factories do (role defaults, display:block emulation, the hidden polyfill) moves out of the hook bodies into plain helpers so it can write to the caller-owned nativeProps without tripping react-rule-hook-mutation. Route HostInstance through the renderer.native type boundary rather than importing it from 'react-native' in each module, matching how the runtime already funnels RN access through the local wrapper. Two suppressions are left in place on purpose: the skew check in useStyleTransition and the provideInheritableStyle comparison both sit on top of latent behavior bugs, so the fixes (and the type cleanup that goes with them) land in a separate PR rather than riding along with a types-only change. * Drop the $FlowFixMe in the web runtime The default style table cast every entry with $FlowFixMe[incompatible-type]; the types line up now, so the casts are gone. For the debug-style object that stylex.create does not generate, use a small local DebugCompiledStyle type instead of an unclear-type cast. The one suppression left on validateStrictProps stays, with a note on why: typing the argument precisely would block the in-place delete of invalid keys that the function relies on. * Add Flow tests for the tightened prop and ref types Now that the props are typed, pin the behaviour down. refs-and-props covers the host-element ref types and the props that should be accepted; expected-errors locks in the misuse Flow has to reject, so a future loosening of the types fails here instead of slipping through. Extend html-types-match with the new event payload types. --------- Co-authored-by: Yamin Yassin <Yamin.Yassin+CAG@cagtechhub.com>
1 parent a6a8c15 commit c877f5c

27 files changed

Lines changed: 955 additions & 718 deletions

‎packages/react-strict-dom/src/native/compat.js‎

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@
88
*/
99

1010
import type { StrictProps } from '../types/StrictProps';
11+
import type { StrictReactDOMProps } from '../types/StrictReactDOMProps';
12+
import type { StrictReactDOMImageProps } from '../types/StrictReactDOMImageProps';
13+
import type { StrictReactDOMInputProps } from '../types/StrictReactDOMInputProps';
14+
import type { StrictReactDOMTextAreaProps } from '../types/StrictReactDOMTextAreaProps';
1115

1216
import * as React from 'react';
1317

@@ -45,14 +49,38 @@ type StrictPropsOnlyCompat<T> = {
4549
children: (nativeProps: T) => React.Node
4650
};
4751

48-
const StrictText = createStrictText('span', defaultProps) as $FlowFixMe;
49-
const StrictInput = createStrictTextInput('input', defaultProps) as $FlowFixMe;
50-
const StrictTextArea = createStrictTextInput(
52+
const StrictText: component(
53+
ref?: React.RefSetter<HTMLSpanElement>,
54+
...StrictReactDOMProps
55+
) = createStrictText<HTMLSpanElement, StrictReactDOMProps>(
56+
'span',
57+
defaultProps
58+
);
59+
const StrictInput: component(
60+
ref?: React.RefSetter<HTMLInputElement>,
61+
...StrictReactDOMInputProps
62+
) = createStrictTextInput<HTMLInputElement, StrictReactDOMInputProps>(
63+
'input',
64+
defaultProps
65+
);
66+
const StrictTextArea: component(
67+
ref?: React.RefSetter<HTMLTextAreaElement>,
68+
...StrictReactDOMTextAreaProps
69+
) = createStrictTextInput<HTMLTextAreaElement, StrictReactDOMTextAreaProps>(
5170
'textarea',
5271
defaultProps
53-
) as $FlowFixMe;
54-
const StrictImage = createStrictImage('img', defaultProps) as $FlowFixMe;
55-
const Strict = createStrict('div', defaultProps) as $FlowFixMe;
72+
);
73+
const StrictImage: component(
74+
ref?: React.RefSetter<HTMLImageElement>,
75+
...StrictReactDOMImageProps
76+
) = createStrictImage<HTMLImageElement, StrictReactDOMImageProps>(
77+
'img',
78+
defaultProps
79+
);
80+
const Strict: component(
81+
ref?: React.RefSetter<HTMLDivElement>,
82+
...StrictReactDOMProps
83+
) = createStrict<HTMLDivElement, StrictReactDOMProps>('div', defaultProps);
5684

5785
component Native<T>(...htmlProps: StrictPropsOnlyCompat<T>) {
5886
const { as, ...rest } = htmlProps;
@@ -61,7 +89,8 @@ component Native<T>(...htmlProps: StrictPropsOnlyCompat<T>) {
6189
'<compat.native> requires the "children" prop to be a function.'
6290
);
6391
}
64-
let Component = Strict;
92+
// Untyped: dispatches across components with divergent prop types.
93+
let Component = Strict as $FlowFixMe;
6594
if (as === 'img') {
6695
Component = StrictImage;
6796
} else if (as === 'input') {

‎packages/react-strict-dom/src/native/css/CSSLengthUnitValue.js‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,22 @@
99

1010
import { warnMsg } from '../../shared/logUtils';
1111

12-
const LENGTH_REGEX = /^(-?[0-9]*[.]?[0-9]+)(em|px|rem|vh|vmax|vmin|vw)$/;
13-
1412
type CSSLengthUnitType = 'em' | 'px' | 'rem' | 'vh' | 'vmax' | 'vmin' | 'vw';
1513

14+
const CSS_LENGTH_UNITS: ReadonlyArray<CSSLengthUnitType> = [
15+
'em',
16+
'px',
17+
'rem',
18+
'vh',
19+
'vmax',
20+
'vmin',
21+
'vw'
22+
];
23+
24+
const LENGTH_REGEX = new RegExp(
25+
`^(-?[0-9]*[.]?[0-9]+)(${CSS_LENGTH_UNITS.join('|')})$`
26+
);
27+
1628
type ResolvePixelValueOptions = Readonly<{
1729
fontScale: number | void,
1830
inheritedFontSize: ?number,
@@ -36,8 +48,15 @@ export class CSSLengthUnitValue {
3648
memoizedValues.set(input, null);
3749
return null;
3850
}
51+
3952
const value = match[1];
40-
const unit: $FlowFixMe = match[2];
53+
const rawUnit = match[2];
54+
const unit = CSS_LENGTH_UNITS.find((u) => u === rawUnit);
55+
if (unit == null) {
56+
memoizedValues.set(input, null);
57+
return null;
58+
}
59+
4160
const parsedFloat: number = parseFloat(value);
4261
const cssLengthUnitValue = new CSSLengthUnitValue(parsedFloat, unit);
4362
memoizedValues.set(input, cssLengthUnitValue);

‎packages/react-strict-dom/src/native/css/index.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ function resolveStyle(
301301

302302
export function props(
303303
this: ResolveStyleOptions,
304-
...style: ReadonlyArray<?{ [key: string]: unknown }>
304+
...style: ReadonlyArray<?{ +[key: string]: unknown }>
305305
): ReactNativeProps {
306306
const options = this;
307307

‎packages/react-strict-dom/src/native/index.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,15 @@ type ProviderProps = Readonly<{
4040
}>;
4141

4242
export type { StaticStyles, StyleTheme, StyleVars, Styles, StylesWithout };
43+
export type {
44+
StrictChangeEvent,
45+
StrictClickEvent,
46+
StrictImageErrorEvent,
47+
StrictImageLoadEvent,
48+
StrictInputEvent,
49+
StrictKeyEvent,
50+
StrictOpaqueEventHandler
51+
} from '../types/StrictReactDOMEvents';
4352

4453
function ThemeProvider(props: ProviderProps): React.Node {
4554
const { children, customProperties } = props;

‎packages/react-strict-dom/src/native/modules/TextString.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ export function TextString(props: Props): React.Node {
3232
});
3333

3434
return (
35+
// strict-dom's wide ReactNativeProps spreads onto RN's exact TextProps;
36+
// harmless extras are ignored at runtime.
3537
// $FlowFixMe[incompatible-type]
3638
<ReactNative.Text {...nativeProps} children={children} />
3739
);

‎packages/react-strict-dom/src/native/modules/createStrictDOMComponent.js‎

Lines changed: 107 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@
77
* @flow strict-local
88
*/
99

10-
import type { ReactNativeProps } from '../../types/renderer.native';
10+
import type { CallbackRef } from '../../types/react';
11+
import type {
12+
HostInstance,
13+
ReactNativeProps
14+
} from '../../types/renderer.native';
1115
import type { StrictProps as StrictPropsOriginal } from '../../types/StrictProps';
1216

1317
import * as React from 'react';
@@ -30,6 +34,99 @@ const AnimatedPressable = ReactNative.Animated.createAnimatedComponent(
3034
ReactNative.Pressable
3135
);
3236

37+
// Plain (non-hook) helper so it can mutate the caller-owned `nativeProps`
38+
// without a defensive copy (a hook may not mutate its own return value).
39+
function applyViewProps(
40+
nativeProps: ReactNativeProps,
41+
tagName: string,
42+
isPressable: boolean,
43+
disabled: boolean,
44+
elementRef: CallbackRef<HostInstance>,
45+
displayInsideValue: 'flow' | 'flex'
46+
): 'flow' | 'flex' {
47+
// Tag-specific props
48+
49+
if (tagName === 'button') {
50+
nativeProps.role ??= 'button';
51+
} else if (tagName === 'header') {
52+
nativeProps.role ??= 'header';
53+
} else if (tagName === 'li') {
54+
nativeProps.role ??= 'listitem';
55+
} else if (tagName === 'ol' || tagName === 'ul') {
56+
nativeProps.role ??= 'list';
57+
}
58+
59+
// Component-specific props
60+
61+
if (isPressable && disabled) {
62+
nativeProps.disabled = true;
63+
nativeProps.focusable = false;
64+
}
65+
66+
nativeProps.ref = elementRef;
67+
68+
// Workaround: React Native doesn't support raw text children of View
69+
// Sometimes we can auto-fix this
70+
if (typeof nativeProps.children === 'string') {
71+
nativeProps.children = <TextString children={nativeProps.children} />;
72+
}
73+
74+
// Polyfill for default of "display:block"
75+
// which implies "displayInside:flow"
76+
let nextDisplayInsideValue: 'flow' | 'flex' = 'flow';
77+
const displayValue = nativeProps.style.display;
78+
79+
if (__DEV__) {
80+
const nativeStyle = nativeProps.style;
81+
if (displayInsideValue !== 'flex') {
82+
// Error message if the element is not a flex child but tries to use flex
83+
['flex', 'flexBasis', 'flexGrow', 'flexShrink'].forEach((styleProp) => {
84+
const value = nativeStyle[styleProp];
85+
if (value != null) {
86+
errorMsg(
87+
`"display:flex" is required on the parent for "${styleProp}" to have an effect.`
88+
);
89+
}
90+
});
91+
// Error message if the element is not a flex child but tries to use
92+
// zIndex without non-static position
93+
if (nativeStyle.zIndex != null && nativeStyle.position === 'static') {
94+
errorMsg(
95+
'"position:static" prevents "zIndex" from having an effect. Try setting "position" to something other than "static".'
96+
);
97+
}
98+
}
99+
}
100+
101+
if (displayValue === 'flex') {
102+
nextDisplayInsideValue = 'flex';
103+
nativeProps.style.alignContent ??= 'stretch';
104+
nativeProps.style.alignItems ??= 'stretch';
105+
nativeProps.style.flexBasis ??= 'auto';
106+
nativeProps.style.flexDirection ??= 'row';
107+
nativeProps.style.flexShrink ??= 1;
108+
nativeProps.style.flexWrap ??= 'nowrap';
109+
nativeProps.style.justifyContent ??= 'flex-start';
110+
} else if (displayValue === 'block' && displayInsideValue === 'flow') {
111+
// Force the block emulation styles
112+
nextDisplayInsideValue = 'flow';
113+
nativeProps.style.alignItems = 'stretch';
114+
nativeProps.style.display = 'flex';
115+
nativeProps.style.flexBasis = 'auto';
116+
nativeProps.style.flexDirection = 'column';
117+
nativeProps.style.flexShrink = 0;
118+
nativeProps.style.flexWrap = 'nowrap';
119+
nativeProps.style.justifyContent = 'flex-start';
120+
}
121+
122+
if (displayInsideValue === 'flex') {
123+
// flex child should not shrink by default
124+
nativeProps.style.flexShrink ??= 1;
125+
}
126+
127+
return nextDisplayInsideValue;
128+
}
129+
33130
export function createStrictDOMComponent<T, P extends StrictProps>(
34131
tagName: string,
35132
defaultProps?: P
@@ -49,6 +146,7 @@ export function createStrictDOMComponent<T, P extends StrictProps>(
49146
: ReactNative.ViewNativeComponent;
50147
const elementRef = useStrictDOMElement<T>(ref, { tagName });
51148
const hasTextAncestor = React.useContext(ReactNative.TextAncestorContext);
149+
const displayInsideValue = useDisplayInside();
52150

53151
/**
54152
* Resolve global HTML and style props
@@ -71,111 +169,14 @@ export function createStrictDOMComponent<T, P extends StrictProps>(
71169
NativeComponent = ReactNative.Pressable;
72170
}
73171

74-
// Tag-specific props
75-
76-
if (tagName === 'button') {
77-
// $FlowFixMe[react-rule-hook-mutation]
78-
nativeProps.role ??= 'button';
79-
} else if (tagName === 'header') {
80-
// $FlowFixMe[react-rule-hook-mutation]
81-
nativeProps.role ??= 'header';
82-
} else if (tagName === 'li') {
83-
// $FlowFixMe[react-rule-hook-mutation]
84-
nativeProps.role ??= 'listitem';
85-
} else if (tagName === 'ol' || tagName === 'ul') {
86-
// $FlowFixMe[react-rule-hook-mutation]
87-
nativeProps.role ??= 'list';
88-
}
89-
90-
// Component-specific props
91-
92-
if (NativeComponent === ReactNative.Pressable) {
93-
if (props.disabled === true) {
94-
// $FlowFixMe[react-rule-hook-mutation]
95-
nativeProps.disabled = true;
96-
// $FlowFixMe[react-rule-hook-mutation]
97-
nativeProps.focusable = false;
98-
}
99-
}
100-
101-
// $FlowFixMe[react-rule-hook-mutation]
102-
nativeProps.ref = elementRef;
103-
104-
// Workaround: React Native doesn't support raw text children of View
105-
// Sometimes we can auto-fix this
106-
if (typeof nativeProps.children === 'string') {
107-
// $FlowFixMe[react-rule-hook-mutation]
108-
nativeProps.children = <TextString children={nativeProps.children} />;
109-
}
110-
111-
// Polyfill for default of "display:block"
112-
// which implies "displayInside:flow"
113-
let nextDisplayInsideValue: 'flow' | 'flex' = 'flow';
114-
const displayInsideValue = useDisplayInside();
115-
const displayValue = nativeProps.style.display;
116-
117-
if (__DEV__) {
118-
const nativeStyle = nativeProps.style;
119-
if (displayInsideValue !== 'flex') {
120-
// Error message if the element is not a flex child but tries to use flex
121-
['flex', 'flexBasis', 'flexGrow', 'flexShrink'].forEach((styleProp) => {
122-
const value = nativeStyle[styleProp];
123-
if (value != null) {
124-
errorMsg(
125-
`"display:flex" is required on the parent for "${styleProp}" to have an effect.`
126-
);
127-
}
128-
});
129-
// Error message if the element is not a flex child but tries to use
130-
// zIndex without non-static position
131-
if (nativeStyle.zIndex != null && nativeStyle.position === 'static') {
132-
errorMsg(
133-
'"position:static" prevents "zIndex" from having an effect. Try setting "position" to something other than "static".'
134-
);
135-
}
136-
}
137-
}
138-
139-
if (displayValue === 'flex') {
140-
nextDisplayInsideValue = 'flex';
141-
// $FlowFixMe[react-rule-hook-mutation]
142-
nativeProps.style.alignContent ??= 'stretch';
143-
// $FlowFixMe[react-rule-hook-mutation]
144-
nativeProps.style.alignItems ??= 'stretch';
145-
// $FlowFixMe[react-rule-hook-mutation]
146-
nativeProps.style.flexBasis ??= 'auto';
147-
// $FlowFixMe[react-rule-hook-mutation]
148-
nativeProps.style.flexDirection ??= 'row';
149-
// $FlowFixMe[react-rule-hook-mutation]
150-
nativeProps.style.flexShrink ??= 1;
151-
// $FlowFixMe[react-rule-hook-mutation]
152-
nativeProps.style.flexWrap ??= 'nowrap';
153-
// $FlowFixMe[react-rule-hook-mutation]
154-
nativeProps.style.justifyContent ??= 'flex-start';
155-
} else if (displayValue === 'block' && displayInsideValue === 'flow') {
156-
// Force the block emulation styles
157-
nextDisplayInsideValue = 'flow';
158-
// $FlowFixMe[react-rule-hook-mutation]
159-
nativeProps.style.alignItems = 'stretch';
160-
// $FlowFixMe[react-rule-hook-mutation]
161-
nativeProps.style.display = 'flex';
162-
// $FlowFixMe[react-rule-hook-mutation]
163-
nativeProps.style.flexBasis = 'auto';
164-
// $FlowFixMe[react-rule-hook-mutation]
165-
nativeProps.style.flexDirection = 'column';
166-
// $FlowFixMe[react-rule-hook-mutation]
167-
nativeProps.style.flexShrink = 0;
168-
// $FlowFixMe[react-rule-hook-mutation]
169-
nativeProps.style.flexWrap = 'nowrap';
170-
// $FlowFixMe[react-rule-hook-mutation]
171-
nativeProps.style.justifyContent = 'flex-start';
172-
}
173-
174-
if (displayInsideValue === 'flex') {
175-
// flex child should not shrink by default
176-
// $FlowFixMe[react-rule-hook-mutation]
177-
nativeProps.style.flexShrink ??= 1;
178-
}
172+
const nextDisplayInsideValue = applyViewProps(
173+
nativeProps,
174+
tagName,
175+
NativeComponent === ReactNative.Pressable,
176+
props.disabled === true,
177+
elementRef,
178+
displayInsideValue
179+
);
179180

180181
// Use Animated components if necessary
181182
if (nativeProps.animated === true) {

0 commit comments

Comments
 (0)