From 2a901e5fb11ac314c753867fbc5b87060ed05f53 Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Wed, 31 May 2023 13:29:52 -0700 Subject: [PATCH] [fix] Image: style resolving Cannot pass the result of StyleSheet.flatten to components, as it mixes dynamic and static (compiled away) style information. With the way Image is currently implemented, we have to override 'boxShadow' in all cases to avoid the 'shadow*' props being incorrectly applied as a box-shadow. Once Image is implemented using createElement, we can disable the 'boxShadow" generation just for Image. Fix #2527 --- package-lock.json | 4 +- packages/react-native-web/package.json | 2 +- .../__snapshots__/index-test.js.snap | 17 ++++++-- .../src/exports/Image/__tests__/index-test.js | 14 ++++++- .../src/exports/Image/index.js | 42 ++++++++++++------- .../src/exports/Image/types.js | 22 +--------- .../src/exports/StyleSheet/index.js | 27 ++++++++---- .../src/exports/StyleSheet/preprocess.js | 15 ++++--- .../src/modules/createDOMProps/index.js | 5 ++- packages/react-native-web/src/types/styles.js | 2 +- 10 files changed, 92 insertions(+), 58 deletions(-) diff --git a/package-lock.json b/package-lock.json index 647be24a..bd98caf1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15490,7 +15490,7 @@ "normalize-css-color": "^1.0.2", "nullthrows": "^1.1.1", "postcss-value-parser": "^4.2.0", - "styleq": "^0.1.2" + "styleq": "^0.1.3" }, "peerDependencies": { "react": "^18.0.0", @@ -23641,7 +23641,7 @@ "normalize-css-color": "^1.0.2", "nullthrows": "^1.1.1", "postcss-value-parser": "^4.2.0", - "styleq": "^0.1.2" + "styleq": "^0.1.3" } }, "react-native-web-docs": { diff --git a/packages/react-native-web/package.json b/packages/react-native-web/package.json index 5088e8d6..831209fa 100644 --- a/packages/react-native-web/package.json +++ b/packages/react-native-web/package.json @@ -29,7 +29,7 @@ "normalize-css-color": "^1.0.2", "nullthrows": "^1.1.1", "postcss-value-parser": "^4.2.0", - "styleq": "^0.1.2" + "styleq": "^0.1.3" }, "peerDependencies": { "react": "^18.0.0", diff --git a/packages/react-native-web/src/exports/Image/__tests__/__snapshots__/index-test.js.snap b/packages/react-native-web/src/exports/Image/__tests__/__snapshots__/index-test.js.snap index 55e2d30a..3ac2c39c 100644 --- a/packages/react-native-web/src/exports/Image/__tests__/__snapshots__/index-test.js.snap +++ b/packages/react-native-web/src/exports/Image/__tests__/__snapshots__/index-test.js.snap @@ -351,7 +351,7 @@ exports[`components/Image prop "style" removes other unsupported View styles 1`] `; -exports[`components/Image prop "style" supports "shadow" properties (convert to filter) 1`] = ` +exports[`components/Image prop "style" supports "shadow" properties (converts to filter) 1`] = `
@@ -362,6 +362,17 @@ exports[`components/Image prop "style" supports "shadow" properties (convert to
`; +exports[`components/Image prop "style" supports static and dynamic styles 1`] = ` +
+
+
+`; + exports[`components/Image prop "testID" 1`] = `
{ }); describe('prop "style"', () => { - test('supports "shadow" properties (convert to filter)', () => { + test('supports "shadow" properties (converts to filter)', () => { const { container } = render( { ); expect(container.firstChild).toMatchSnapshot(); }); + + test('supports static and dynamic styles', () => { + const { container } = render( + + ); + expect(container.firstChild).toMatchSnapshot(); + }); }); describe('prop "tintColor"', () => { diff --git a/packages/react-native-web/src/exports/Image/index.js b/packages/react-native-web/src/exports/Image/index.js index bd69e5e8..a447968d 100644 --- a/packages/react-native-web/src/exports/Image/index.js +++ b/packages/react-native-web/src/exports/Image/index.js @@ -51,7 +51,12 @@ function createTintColorSVG(tintColor, id) { ) : null; } -function getFlatStyle(style, blurRadius, filterId, tintColorProp) { +function extractNonStandardStyleProps( + style, + blurRadius, + filterId, + tintColorProp +) { const flatStyle = StyleSheet.flatten(style); const { filter, resizeMode, shadowOffset, tintColor } = flatStyle; @@ -93,19 +98,7 @@ function getFlatStyle(style, blurRadius, filterId, tintColorProp) { _filter = filters.join(' '); } - // These styles are converted to CSS filters applied to the - // element displaying the background image. - delete flatStyle.blurRadius; - delete flatStyle.shadowColor; - delete flatStyle.shadowOpacity; - delete flatStyle.shadowOffset; - delete flatStyle.shadowRadius; - delete flatStyle.tintColor; - // These styles are not supported on View - delete flatStyle.overlayColor; - delete flatStyle.resizeMode; - - return [flatStyle, resizeMode, _filter, tintColor]; + return [resizeMode, _filter, tintColor]; } function resolveAssetDimensions(source) { @@ -223,7 +216,7 @@ const Image: React.AbstractComponent< const requestRef = React.useRef(null); const shouldDisplaySource = state === LOADED || (state === LOADING && defaultSource == null); - const [flatStyle, _resizeMode, filter, _tintColor] = getFlatStyle( + const [_resizeMode, filter, _tintColor] = extractNonStandardStyleProps( style, blurRadius, filterRef.current, @@ -335,7 +328,11 @@ const Image: React.AbstractComponent< styles.root, hasTextAncestor && styles.inline, imageSizeStyle, - flatStyle + style, + styles.undo, + // TEMP: avoid deprecated shadow props regression + // until Image refactored to use createElement. + { boxShadow: null } ]} > ; export type ImageStyle = { - ...AnimationStyles, - ...BorderStyles, - ...InteractionStyles, - ...LayoutStyles, - ...ShadowStyles, - ...TransformStyles, - backgroundColor?: ColorValue, - boxShadow?: string, - filter?: string, - opacity?: number, + ...ViewStyle, // @deprecated resizeMode?: ResizeMode, tintColor?: ColorValue diff --git a/packages/react-native-web/src/exports/StyleSheet/index.js b/packages/react-native-web/src/exports/StyleSheet/index.js index 89bacb89..12781c69 100644 --- a/packages/react-native-web/src/exports/StyleSheet/index.js +++ b/packages/react-native-web/src/exports/StyleSheet/index.js @@ -18,14 +18,21 @@ import canUseDOM from '../../modules/canUseDom'; const staticStyleMap: WeakMap = new WeakMap(); const sheet = createSheet(); -function customStyleq(styles, isRTL) { +const defaultPreprocessOptions = { shadow: true, textShadow: true }; + +function customStyleq(styles, options: Options = {}) { + const { writingDirection, ...preprocessOptions } = options; + const isRTL = writingDirection === 'rtl'; return styleq.factory({ transform(style) { const compiledStyle = staticStyleMap.get(style); if (compiledStyle != null) { return localizeStyle(compiledStyle, isRTL); } - return preprocess(style); + return preprocess(style, { + ...defaultPreprocessOptions, + ...preprocessOptions + }); } })(styles); } @@ -41,7 +48,9 @@ function insertRules(compiledOrderedRules) { } function compileAndInsertAtomic(style) { - const [compiledStyle, compiledOrderedRules] = atomic(preprocess(style)); + const [compiledStyle, compiledOrderedRules] = atomic( + preprocess(style, defaultPreprocessOptions) + ); insertRules(compiledOrderedRules); return compiledStyle; } @@ -141,11 +150,15 @@ function getSheet(): { id: string, textContent: string } { * resolve */ type StyleProps = [string, { [key: string]: mixed } | null]; -type Options = { writingDirection: 'ltr' | 'rtl' }; +type Options = { + shadow?: boolean, + textShadow?: boolean, + writingDirection: 'ltr' | 'rtl' +}; -function StyleSheet(styles: any, options?: Options): StyleProps { - const isRTL = options != null && options.writingDirection === 'rtl'; - const styleProps: StyleProps = customStyleq(styles, isRTL); +function StyleSheet(styles: any, options?: Options = {}): StyleProps { + const isRTL = options.writingDirection === 'rtl'; + const styleProps: StyleProps = customStyleq(styles, options); if (Array.isArray(styleProps) && styleProps[1] != null) { styleProps[1] = inline(styleProps[1], isRTL); } diff --git a/packages/react-native-web/src/exports/StyleSheet/preprocess.js b/packages/react-native-web/src/exports/StyleSheet/preprocess.js index 3ff077f9..4b3d1487 100644 --- a/packages/react-native-web/src/exports/StyleSheet/preprocess.js +++ b/packages/react-native-web/src/exports/StyleSheet/preprocess.js @@ -107,17 +107,19 @@ const ignoredProps = { * Preprocess styles */ export const preprocess = ( - originalStyle: T + originalStyle: T, + options?: { shadow?: boolean, textShadow?: boolean } = {} ): T => { const style = originalStyle || emptyObject; const nextStyle = {}; // Convert shadow styles if ( + (options.shadow === true, style.shadowColor != null || - style.shadowOffset != null || - style.shadowOpacity != null || - style.shadowRadius != null + style.shadowOffset != null || + style.shadowOpacity != null || + style.shadowRadius != null) ) { warnOnce( 'shadowStyles', @@ -135,9 +137,10 @@ export const preprocess = ( // Convert text shadow styles if ( + (options.textShadow === true, style.textShadowColor != null || - style.textShadowOffset != null || - style.textShadowRadius != null + style.textShadowOffset != null || + style.textShadowRadius != null) ) { warnOnce( 'textShadowStyles', diff --git a/packages/react-native-web/src/modules/createDOMProps/index.js b/packages/react-native-web/src/modules/createDOMProps/index.js index eaa7763f..d6386d8f 100644 --- a/packages/react-native-web/src/modules/createDOMProps/index.js +++ b/packages/react-native-web/src/modules/createDOMProps/index.js @@ -787,7 +787,10 @@ const createDOMProps = (elementType, props, options) => { } const [className, inlineStyle] = StyleSheet( [style, pointerEvents && pointerEventsStyles[pointerEvents]], - { writingDirection: options ? options.writingDirection : 'ltr' } + { + writingDirection: 'ltr', + ...options + } ); if (className) { domProps.className = className; diff --git a/packages/react-native-web/src/types/styles.js b/packages/react-native-web/src/types/styles.js index b9e37147..296a2abf 100644 --- a/packages/react-native-web/src/types/styles.js +++ b/packages/react-native-web/src/types/styles.js @@ -306,7 +306,7 @@ export type LayoutStyles = {| export type ShadowStyles = {| // @deprecated shadowColor?: ?ColorValue, - shadowOffset?: {| + shadowOffset?: ?{| width?: DimensionValue, height?: DimensionValue |},