From 77d18886ee02b5057ced73c89e2ea18ef7d98086 Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Sat, 16 Aug 2025 17:27:51 -0400 Subject: [PATCH] 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. --- .../react-strict-dom/src/native/css/index.js | 56 ++++++++++++------- .../modules/createStrictDOMComponent.js | 24 ++++---- .../src/native/modules/useNativeProps.js | 10 ---- .../react-strict-dom/tests/css-test.native.js | 8 +-- .../tests/html-test.native.js | 49 ++++++++++++++++ 5 files changed, 101 insertions(+), 46 deletions(-) diff --git a/packages/react-strict-dom/src/native/css/index.js b/packages/react-strict-dom/src/native/css/index.js index 67c47d57..931cb56e 100644 --- a/packages/react-strict-dom/src/native/css/index.js +++ b/packages/react-strict-dom/src/native/css/index.js @@ -16,7 +16,7 @@ import type { IStyleX } from '../../types/styles'; import { CSSLengthUnitValue } from './CSSLengthUnitValue'; import { CSSTransformValue } from './CSSTransformValue'; import { CSSUnparsedValue } from './typed-om/CSSUnparsedValue'; -import { errorMsg } from '../../shared/logUtils'; +import { errorMsg, warnMsg } from '../../shared/logUtils'; import { flattenStyle } from './flattenStyleXStyles'; import { lengthStyleKeySet } from './isLengthStyleKey'; import { mediaQueryMatches } from './mediaQueryMatches'; @@ -475,28 +475,44 @@ export function props( } } - // Print an error message if flex properties are used without - // "display:flex" being set. React Native is always using "flex" - // layout but web uses "flow" layout by default, which can lead - // to layout divergence if building for native first. if (__DEV__) { + const displayValue = nextStyle.display; + + // Warning message if using unsupported display style if ( - nextStyle.display == null || - (nextStyle.display !== 'flex' && nextStyle.display !== 'none') + displayValue != null && + displayValue !== 'flex' && + displayValue !== 'none' && + displayValue !== 'block' ) { - if ( - nextStyle.alignContent != null || - nextStyle.alignItems != null || - nextStyle.columnGap != null || - nextStyle.flexDirection != null || - nextStyle.flexWrap != null || - nextStyle.gap != null || - nextStyle.justifyContent != null || - nextStyle.placeContent != null || - nextStyle.rowGap != null - ) { - errorMsg('"display:flex" is required to use flexbox properties'); - } + warnMsg(`"display:${String(displayValue)}" is not a supported value`); + } + + // Error message if using flex properties without "display:flex" + // being set. React Native is always using "flex" layout but web + // uses "flow" layout by default, which can lead to layout + // divergence if building for native first. + if ( + displayValue == null || + (displayValue !== 'flex' && displayValue !== 'none') + ) { + [ + 'alignContent', + 'alignItems', + 'columnGap', + 'flexDirection', + 'flexWrap', + 'gap', + 'justifyContent', + 'rowGap' + ].forEach((styleProp) => { + const value = nextStyle[styleProp]; + if (value != null) { + errorMsg( + `"display:flex" is required for "${styleProp}" to have an effect.` + ); + } + }); } } diff --git a/packages/react-strict-dom/src/native/modules/createStrictDOMComponent.js b/packages/react-strict-dom/src/native/modules/createStrictDOMComponent.js index 26157686..23d63fb2 100644 --- a/packages/react-strict-dom/src/native/modules/createStrictDOMComponent.js +++ b/packages/react-strict-dom/src/native/modules/createStrictDOMComponent.js @@ -17,7 +17,7 @@ import { ProvideCustomProperties } from './ContextCustomProperties'; import { ProvideDisplayInside, useDisplayInside } from './ContextDisplayInside'; import { ProvideInheritedStyles } from './ContextInheritedStyles'; import { TextString } from './TextString'; -import { warnMsg } from '../../shared/logUtils'; +import { errorMsg } from '../../shared/logUtils'; import { useNativeProps } from './useNativeProps'; import { useStrictDOMElement } from './useStrictDOMElement'; @@ -99,14 +99,18 @@ export function createStrictDOMComponent( const displayValue = nativeProps.style.display; if (__DEV__) { - if ( - displayValue != null && - displayValue !== 'flex' && - displayValue !== 'none' && - displayValue !== 'block' - ) { - warnMsg( - `unsupported style value in "display:${String(displayValue)}"` + const nativeStyle = nativeProps.style; + if (displayInsideValue !== 'flex') { + // Error message if the element is not a flex child but tries to use flex + ['flex', 'flexBasis', 'flexGrow', 'flexShrink'].forEach( + (styleProp) => { + const value = nativeStyle[styleProp]; + if (value != null) { + errorMsg( + `"display:flex" is required on the parent for "${styleProp}" to have an effect.` + ); + } + } ); } } @@ -133,7 +137,7 @@ export function createStrictDOMComponent( } if (displayInsideValue === 'flex') { - // flex child should not shrink + // flex child should not shrink by default nativeProps.style.flexShrink ??= 1; } diff --git a/packages/react-strict-dom/src/native/modules/useNativeProps.js b/packages/react-strict-dom/src/native/modules/useNativeProps.js index ab2cb4fe..26f173e8 100644 --- a/packages/react-strict-dom/src/native/modules/useNativeProps.js +++ b/packages/react-strict-dom/src/native/modules/useNativeProps.js @@ -167,16 +167,6 @@ export function useNativeProps( }); const displayValue = nativeProps.style.display; - if (__DEV__) { - if ( - displayValue != null && - displayValue !== 'flex' && - displayValue !== 'none' && - displayValue !== 'block' - ) { - warnMsg(`unsupported style value in "display:${String(displayValue)}"`); - } - } // 'hidden' polyfill (only if "display" is not set) if (displayValue == null && hidden && hidden !== 'until-found') { nativeProps.style.display = 'none'; diff --git a/packages/react-strict-dom/tests/css-test.native.js b/packages/react-strict-dom/tests/css-test.native.js index 10c2b7d0..4519250b 100644 --- a/packages/react-strict-dom/tests/css-test.native.js +++ b/packages/react-strict-dom/tests/css-test.native.js @@ -266,15 +266,11 @@ describe('properties: general', () => { }); css.props.call(mockOptions, [styles.flex, styles.align]); expect(console.error).not.toHaveBeenCalledWith( - expect.stringContaining( - '"display:flex" is required to use flexbox properties' - ) + expect.stringContaining('"display:flex" is required') ); css.props.call(mockOptions, [styles.align, styles.row]); expect(console.error).toHaveBeenCalledWith( - expect.stringContaining( - '"display:flex" is required to use flexbox properties' - ) + expect.stringContaining('"display:flex" is required') ); }); diff --git a/packages/react-strict-dom/tests/html-test.native.js b/packages/react-strict-dom/tests/html-test.native.js index b2d1cad8..d6c93d02 100644 --- a/packages/react-strict-dom/tests/html-test.native.js +++ b/packages/react-strict-dom/tests/html-test.native.js @@ -89,6 +89,55 @@ describe('', () => { expect(root.toJSON()).toMatchSnapshot('block layout override of flex'); }); + [ + 'alignContent', + 'alignItems', + 'columnGap', + 'flexDirection', + 'flexWrap', + 'gap', + 'justifyContent', + 'rowGap' + ].forEach((styleProp) => { + test(`block layout error: "${styleProp}"`, () => { + const styles = css.create({ + root: { + [styleProp]: 'center' + } + }); + act(() => { + create(); + }); + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining( + `"display:flex" is required for "${styleProp}" to have an effect.` + ) + ); + }); + }); + + ['flex', 'flexBasis', 'flexGrow', 'flexShrink'].forEach((styleProp) => { + test(`block layout error for flex child: "${styleProp}"`, () => { + const styles = css.create({ + root: { + [styleProp]: 1 + } + }); + act(() => { + create( + + + + ); + }); + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining( + `"display:flex" is required on the parent for "${styleProp}" to have an effect.` + ) + ); + }); + }); + test('auto-wraps raw strings', () => { const styles = css.create({ root: {