Skip to content

Commit c8e3bee

Browse files
necolasnecolas
committed
Add error messages for styles that only work on flex children
These messages will help developers on native to know when styles will have no effect on web because they only work on flex children, but the parent is not a flex container. On native, the styles will work because all elements are `display:flex`, but this is not the case on web.
1 parent e7c1468 commit c8e3bee

File tree

5 files changed

+101
-46
lines changed

5 files changed

+101
-46
lines changed

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

Lines changed: 36 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import type { IStyleX } from '../../types/styles';
1616
import { CSSLengthUnitValue } from './CSSLengthUnitValue';
1717
import { CSSTransformValue } from './CSSTransformValue';
1818
import { CSSUnparsedValue } from './typed-om/CSSUnparsedValue';
19-
import { errorMsg } from '../../shared/logUtils';
19+
import { errorMsg, warnMsg } from '../../shared/logUtils';
2020
import { flattenStyle } from './flattenStyleXStyles';
2121
import { lengthStyleKeySet } from './isLengthStyleKey';
2222
import { mediaQueryMatches } from './mediaQueryMatches';
@@ -475,28 +475,44 @@ export function props(
475475
}
476476
}
477477

478-
// Print an error message if flex properties are used without
479-
// "display:flex" being set. React Native is always using "flex"
480-
// layout but web uses "flow" layout by default, which can lead
481-
// to layout divergence if building for native first.
482478
if (__DEV__) {
479+
const displayValue = nextStyle.display;
480+
481+
// Warning message if using unsupported display style
483482
if (
484-
nextStyle.display == null ||
485-
(nextStyle.display !== 'flex' && nextStyle.display !== 'none')
483+
displayValue != null &&
484+
displayValue !== 'flex' &&
485+
displayValue !== 'none' &&
486+
displayValue !== 'block'
486487
) {
487-
if (
488-
nextStyle.alignContent != null ||
489-
nextStyle.alignItems != null ||
490-
nextStyle.columnGap != null ||
491-
nextStyle.flexDirection != null ||
492-
nextStyle.flexWrap != null ||
493-
nextStyle.gap != null ||
494-
nextStyle.justifyContent != null ||
495-
nextStyle.placeContent != null ||
496-
nextStyle.rowGap != null
497-
) {
498-
errorMsg('"display:flex" is required to use flexbox properties');
499-
}
488+
warnMsg(`"display:${String(displayValue)}" is not a supported value`);
489+
}
490+
491+
// Error message if using flex properties without "display:flex"
492+
// being set. React Native is always using "flex" layout but web
493+
// uses "flow" layout by default, which can lead to layout
494+
// divergence if building for native first.
495+
if (
496+
displayValue == null ||
497+
(displayValue !== 'flex' && displayValue !== 'none')
498+
) {
499+
[
500+
'alignContent',
501+
'alignItems',
502+
'columnGap',
503+
'flexDirection',
504+
'flexWrap',
505+
'gap',
506+
'justifyContent',
507+
'rowGap'
508+
].forEach((styleProp) => {
509+
const value = nextStyle[styleProp];
510+
if (value != null) {
511+
errorMsg(
512+
`"display:flex" is required for "${styleProp}" to have an effect.`
513+
);
514+
}
515+
});
500516
}
501517
}
502518

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

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import { ProvideCustomProperties } from './ContextCustomProperties';
1717
import { ProvideDisplayInside, useDisplayInside } from './ContextDisplayInside';
1818
import { ProvideInheritedStyles } from './ContextInheritedStyles';
1919
import { TextString } from './TextString';
20-
import { warnMsg } from '../../shared/logUtils';
20+
import { errorMsg } from '../../shared/logUtils';
2121
import { useNativeProps } from './useNativeProps';
2222
import { useStrictDOMElement } from './useStrictDOMElement';
2323

@@ -99,14 +99,18 @@ export function createStrictDOMComponent<T, P: StrictProps>(
9999
const displayValue = nativeProps.style.display;
100100

101101
if (__DEV__) {
102-
if (
103-
displayValue != null &&
104-
displayValue !== 'flex' &&
105-
displayValue !== 'none' &&
106-
displayValue !== 'block'
107-
) {
108-
warnMsg(
109-
`unsupported style value in "display:${String(displayValue)}"`
102+
const nativeStyle = nativeProps.style;
103+
if (displayInsideValue !== 'flex') {
104+
// Error message if the element is not a flex child but tries to use flex
105+
['flex', 'flexBasis', 'flexGrow', 'flexShrink'].forEach(
106+
(styleProp) => {
107+
const value = nativeStyle[styleProp];
108+
if (value != null) {
109+
errorMsg(
110+
`"display:flex" is required on the parent for "${styleProp}" to have an effect.`
111+
);
112+
}
113+
}
110114
);
111115
}
112116
}
@@ -133,7 +137,7 @@ export function createStrictDOMComponent<T, P: StrictProps>(
133137
}
134138

135139
if (displayInsideValue === 'flex') {
136-
// flex child should not shrink
140+
// flex child should not shrink by default
137141
nativeProps.style.flexShrink ??= 1;
138142
}
139143

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

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -167,16 +167,6 @@ export function useNativeProps(
167167
});
168168

169169
const displayValue = nativeProps.style.display;
170-
if (__DEV__) {
171-
if (
172-
displayValue != null &&
173-
displayValue !== 'flex' &&
174-
displayValue !== 'none' &&
175-
displayValue !== 'block'
176-
) {
177-
warnMsg(`unsupported style value in "display:${String(displayValue)}"`);
178-
}
179-
}
180170
// 'hidden' polyfill (only if "display" is not set)
181171
if (displayValue == null && hidden && hidden !== 'until-found') {
182172
nativeProps.style.display = 'none';

‎packages/react-strict-dom/tests/css-test.native.js

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -266,15 +266,11 @@ describe('properties: general', () => {
266266
});
267267
css.props.call(mockOptions, [styles.flex, styles.align]);
268268
expect(console.error).not.toHaveBeenCalledWith(
269-
expect.stringContaining(
270-
'"display:flex" is required to use flexbox properties'
271-
)
269+
expect.stringContaining('"display:flex" is required')
272270
);
273271
css.props.call(mockOptions, [styles.align, styles.row]);
274272
expect(console.error).toHaveBeenCalledWith(
275-
expect.stringContaining(
276-
'"display:flex" is required to use flexbox properties'
277-
)
273+
expect.stringContaining('"display:flex" is required')
278274
);
279275
});
280276

‎packages/react-strict-dom/tests/html-test.native.js

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,55 @@ describe('<html.*>', () => {
8989
expect(root.toJSON()).toMatchSnapshot('block layout override of flex');
9090
});
9191

92+
[
93+
'alignContent',
94+
'alignItems',
95+
'columnGap',
96+
'flexDirection',
97+
'flexWrap',
98+
'gap',
99+
'justifyContent',
100+
'rowGap'
101+
].forEach((styleProp) => {
102+
test(`block layout error: "${styleProp}"`, () => {
103+
const styles = css.create({
104+
root: {
105+
[styleProp]: 'center'
106+
}
107+
});
108+
act(() => {
109+
create(<html.div style={styles.root} />);
110+
});
111+
expect(console.error).toHaveBeenCalledWith(
112+
expect.stringContaining(
113+
`"display:flex" is required for "${styleProp}" to have an effect.`
114+
)
115+
);
116+
});
117+
});
118+
119+
['flex', 'flexBasis', 'flexGrow', 'flexShrink'].forEach((styleProp) => {
120+
test(`block layout error for flex child: "${styleProp}"`, () => {
121+
const styles = css.create({
122+
root: {
123+
[styleProp]: 1
124+
}
125+
});
126+
act(() => {
127+
create(
128+
<html.div>
129+
<html.div style={styles.root} />
130+
</html.div>
131+
);
132+
});
133+
expect(console.error).toHaveBeenCalledWith(
134+
expect.stringContaining(
135+
`"display:flex" is required on the parent for "${styleProp}" to have an effect.`
136+
)
137+
);
138+
});
139+
});
140+
92141
test('auto-wraps raw strings', () => {
93142
const styles = css.create({
94143
root: {

0 commit comments

Comments
 (0)