Skip to content

Commit a7ad793

Browse files
NickGerlemanmeta-codesync[bot]
authored andcommitted
Reland: Wire native CSS parsing for aspectRatio (#55882)
Summary: Pull Request resolved: #55882 ## Reland `convertRawProp` for `yoga::Style` did not preserve `sourceValue.aspectRatio()` when `aspectRatio` was absent from the raw props diff. This caused `aspectRatio` to silently reset to undefined whenever any other prop was updated on a component that had `aspectRatio` set. Fixed by adding an else-branch fallback to `sourceValue.aspectRatio()`: ``` { const auto *rawValue = rawProps.at("aspectRatio", nullptr, nullptr); if (rawValue != nullptr) { yogaStyle.setAspectRatio(rawValue->hasValue() ? convertAspectRatio(context, *rawValue) : yogaStyle.aspectRatio()); + } else { + yogaStyle.setAspectRatio(sourceValue.aspectRatio()); } } ``` Added `clonedPropsPreserveAspectRatio` test in ViewTest.cpp to verify aspectRatio survives a prop clone with empty RawProps. ## Original Gate `processAspectRatio` behind `enableNativeCSSParsing()`. When the flag is on, CSS ratio strings like `"16/9"` and number strings are parsed natively using the existing CSS ratio parser instead of being preprocessed in JS. The parsing is done in `fromRawValue(... FloatOptional &)` — string values are only sent for aspectRatio; other FloatOptional yoga props never receive strings from JS. Changelog: [Internal] Reviewed By: javache Differential Revision: D95032494 fbshipit-source-id: f3c9724628d8ea7e570d1c91ddc38a6b59146070
1 parent 2f6c744 commit a7ad793

File tree

7 files changed

+252
-31
lines changed

7 files changed

+252
-31
lines changed

packages/react-native/Libraries/Components/View/ReactNativeStyleAttributes.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,18 @@ export const fontVariantAttribute: AnyAttributeType = nativeCSSParsing
7171
? true
7272
: {process: processFontVariant};
7373

74+
export const aspectRatioAttribute: AnyAttributeType = nativeCSSParsing
75+
? true
76+
: {process: processAspectRatio};
77+
7478
const ReactNativeStyleAttributes: {[string]: AnyAttributeType, ...} = {
7579
/**
7680
* Layout
7781
*/
7882
alignContent: true,
7983
alignItems: true,
8084
alignSelf: true,
81-
aspectRatio: {process: processAspectRatio},
85+
aspectRatio: aspectRatioAttribute,
8286
borderBottomWidth: true,
8387
borderEndWidth: true,
8488
borderLeftWidth: true,

packages/react-native/Libraries/Components/View/__tests__/View-itest.js

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,101 @@ describe('<View>', () => {
231231
});
232232
});
233233

234+
describe('aspectRatio', () => {
235+
it('is preserved when updating an unrelated prop', () => {
236+
const root = Fantom.createRoot();
237+
238+
Fantom.runTask(() => {
239+
root.render(
240+
<View
241+
style={{width: 100, aspectRatio: 2}}
242+
nativeID="first"
243+
collapsable={false}
244+
/>,
245+
);
246+
});
247+
248+
// width=100, aspectRatio=2 → height = 100 / 2 = 50
249+
expect(
250+
root
251+
.getRenderedOutput({
252+
includeLayoutMetrics: true,
253+
props: ['layoutMetrics-frame'],
254+
})
255+
.toJSX(),
256+
).toEqual(
257+
<rn-view layoutMetrics-frame="{x:0,y:0,width:100,height:50}" />,
258+
);
259+
260+
// Update only nativeID, not aspectRatio
261+
Fantom.runTask(() => {
262+
root.render(
263+
<View
264+
style={{width: 100, aspectRatio: 2}}
265+
nativeID="second"
266+
collapsable={false}
267+
/>,
268+
);
269+
});
270+
271+
// aspectRatio must still be preserved → same layout
272+
expect(
273+
root
274+
.getRenderedOutput({
275+
includeLayoutMetrics: true,
276+
props: ['layoutMetrics-frame'],
277+
})
278+
.toJSX(),
279+
).toEqual(
280+
<rn-view layoutMetrics-frame="{x:0,y:0,width:100,height:50}" />,
281+
);
282+
});
283+
284+
it('can be changed to undefined after initially having a value', () => {
285+
const root = Fantom.createRoot();
286+
287+
Fantom.runTask(() => {
288+
root.render(
289+
<View style={{width: 100, aspectRatio: 2}} collapsable={false} />,
290+
);
291+
});
292+
293+
// width=100, aspectRatio=2 → height = 100 / 2 = 50
294+
expect(
295+
root
296+
.getRenderedOutput({
297+
includeLayoutMetrics: true,
298+
props: ['layoutMetrics-frame'],
299+
})
300+
.toJSX(),
301+
).toEqual(
302+
<rn-view layoutMetrics-frame="{x:0,y:0,width:100,height:50}" />,
303+
);
304+
305+
// Update aspectRatio to undefined
306+
Fantom.runTask(() => {
307+
root.render(
308+
<View
309+
style={{width: 100, aspectRatio: undefined}}
310+
collapsable={false}
311+
/>,
312+
);
313+
});
314+
315+
// aspectRatio is now undefined → height collapses to 0
316+
expect(
317+
root
318+
.getRenderedOutput({
319+
includeLayoutMetrics: true,
320+
props: ['layoutMetrics-frame'],
321+
})
322+
.toJSX(),
323+
).toEqual(
324+
<rn-view layoutMetrics-frame="{x:0,y:0,width:100,height:0}" />,
325+
);
326+
});
327+
});
328+
234329
describe('background-image', () => {
235330
it('it parses CSS and object syntax', () => {
236331
const root = Fantom.createRoot();

packages/react-native/ReactCommon/react/renderer/components/view/YogaStylableProps.cpp

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -147,35 +147,40 @@ void YogaStylableProps::setProp(
147147
REBUILD_FIELD_SWITCH_CASE_YSP(flexBasis, setFlexBasis);
148148
REBUILD_FIELD_SWITCH_CASE2(positionType, setPositionType, "position");
149149
REBUILD_FIELD_YG_GUTTER(gap, setGap, "rowGap", "columnGap", "gap");
150-
REBUILD_FIELD_SWITCH_CASE_YSP(aspectRatio, setAspectRatio);
151-
REBUILD_FIELD_SWITCH_CASE_YSP(boxSizing, setBoxSizing);
152-
REBUILD_FIELD_YG_DIMENSION(dimension, setDimension, "width", "height");
153-
REBUILD_FIELD_YG_DIMENSION(
154-
minDimension, setMinDimension, "minWidth", "minHeight");
155-
REBUILD_FIELD_YG_DIMENSION(
156-
maxDimension, setMaxDimension, "maxWidth", "maxHeight");
157-
REBUILD_FIELD_YG_EDGES_POSITION();
158-
REBUILD_FIELD_YG_EDGES(margin, setMargin, "margin", "");
159-
REBUILD_FIELD_YG_EDGES(padding, setPadding, "padding", "");
160-
REBUILD_FIELD_YG_EDGES(border, setBorder, "border", "Width");
150+
case CONSTEXPR_RAW_PROPS_KEY_HASH("aspectRatio"): {
151+
yogaStyle.setAspectRatio(
152+
value.hasValue() ? convertAspectRatio(context, value)
153+
: ygDefaults.aspectRatio());
154+
return;
155+
}
156+
REBUILD_FIELD_SWITCH_CASE_YSP(boxSizing, setBoxSizing);
157+
REBUILD_FIELD_YG_DIMENSION(dimension, setDimension, "width", "height");
158+
REBUILD_FIELD_YG_DIMENSION(
159+
minDimension, setMinDimension, "minWidth", "minHeight");
160+
REBUILD_FIELD_YG_DIMENSION(
161+
maxDimension, setMaxDimension, "maxWidth", "maxHeight");
162+
REBUILD_FIELD_YG_EDGES_POSITION();
163+
REBUILD_FIELD_YG_EDGES(margin, setMargin, "margin", "");
164+
REBUILD_FIELD_YG_EDGES(padding, setPadding, "padding", "");
165+
REBUILD_FIELD_YG_EDGES(border, setBorder, "border", "Width");
161166

162-
// Aliases
163-
RAW_SET_PROP_SWITCH_CASE(insetBlockEnd, "insetBlockEnd");
164-
RAW_SET_PROP_SWITCH_CASE(insetBlockStart, "insetBlockStart");
165-
RAW_SET_PROP_SWITCH_CASE(insetInlineEnd, "insetInlineEnd");
166-
RAW_SET_PROP_SWITCH_CASE(insetInlineStart, "insetInlineStart");
167-
RAW_SET_PROP_SWITCH_CASE(marginInline, "marginInline");
168-
RAW_SET_PROP_SWITCH_CASE(marginInlineStart, "marginInlineStart");
169-
RAW_SET_PROP_SWITCH_CASE(marginInlineEnd, "marginInlineEnd");
170-
RAW_SET_PROP_SWITCH_CASE(marginBlock, "marginBlock");
171-
RAW_SET_PROP_SWITCH_CASE(marginBlockStart, "marginBlockStart");
172-
RAW_SET_PROP_SWITCH_CASE(marginBlockEnd, "marginBlockEnd");
173-
RAW_SET_PROP_SWITCH_CASE(paddingInline, "paddingInline");
174-
RAW_SET_PROP_SWITCH_CASE(paddingInlineStart, "paddingInlineStart");
175-
RAW_SET_PROP_SWITCH_CASE(paddingInlineEnd, "paddingInlineEnd");
176-
RAW_SET_PROP_SWITCH_CASE(paddingBlock, "paddingBlock");
177-
RAW_SET_PROP_SWITCH_CASE(paddingBlockStart, "paddingBlockStart");
178-
RAW_SET_PROP_SWITCH_CASE(paddingBlockEnd, "paddingBlockEnd");
167+
// Aliases
168+
RAW_SET_PROP_SWITCH_CASE(insetBlockEnd, "insetBlockEnd");
169+
RAW_SET_PROP_SWITCH_CASE(insetBlockStart, "insetBlockStart");
170+
RAW_SET_PROP_SWITCH_CASE(insetInlineEnd, "insetInlineEnd");
171+
RAW_SET_PROP_SWITCH_CASE(insetInlineStart, "insetInlineStart");
172+
RAW_SET_PROP_SWITCH_CASE(marginInline, "marginInline");
173+
RAW_SET_PROP_SWITCH_CASE(marginInlineStart, "marginInlineStart");
174+
RAW_SET_PROP_SWITCH_CASE(marginInlineEnd, "marginInlineEnd");
175+
RAW_SET_PROP_SWITCH_CASE(marginBlock, "marginBlock");
176+
RAW_SET_PROP_SWITCH_CASE(marginBlockStart, "marginBlockStart");
177+
RAW_SET_PROP_SWITCH_CASE(marginBlockEnd, "marginBlockEnd");
178+
RAW_SET_PROP_SWITCH_CASE(paddingInline, "paddingInline");
179+
RAW_SET_PROP_SWITCH_CASE(paddingInlineStart, "paddingInlineStart");
180+
RAW_SET_PROP_SWITCH_CASE(paddingInlineEnd, "paddingInlineEnd");
181+
RAW_SET_PROP_SWITCH_CASE(paddingBlock, "paddingBlock");
182+
RAW_SET_PROP_SWITCH_CASE(paddingBlockStart, "paddingBlockStart");
183+
RAW_SET_PROP_SWITCH_CASE(paddingBlockEnd, "paddingBlockEnd");
179184
}
180185
}
181186

packages/react-native/ReactCommon/react/renderer/components/view/conversions.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,23 @@ inline void fromRawValue(const PropsParserContext &context, const RawValue &valu
498498
result = value.hasType<float>() ? yoga::FloatOptional((float)value) : yoga::FloatOptional();
499499
}
500500

501+
inline yoga::FloatOptional convertAspectRatio(const PropsParserContext & /*context*/, const RawValue &value)
502+
{
503+
if (value.hasType<float>()) {
504+
return yoga::FloatOptional((float)value);
505+
}
506+
if (ReactNativeFeatureFlags::enableNativeCSSParsing() && value.hasType<std::string>()) {
507+
auto ratio = parseCSSProperty<CSSRatio>((std::string)value);
508+
if (std::holds_alternative<CSSRatio>(ratio)) {
509+
auto r = std::get<CSSRatio>(ratio);
510+
if (!r.isDegenerate()) {
511+
return yoga::FloatOptional(r.numerator / r.denominator);
512+
}
513+
}
514+
}
515+
return {};
516+
}
517+
501518
inline std::optional<Float> toRadians(const RawValue &value)
502519
{
503520
if (value.hasType<Float>()) {

packages/react-native/ReactCommon/react/renderer/components/view/propsConversions.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,8 +356,14 @@ convertRawProp(const PropsParserContext &context, const RawProps &rawProps, cons
356356
yoga::Dimension::Height,
357357
convertRawProp(context, rawProps, "maxHeight", sourceValue.maxDimension(yoga::Dimension::Height), {}));
358358

359-
yogaStyle.setAspectRatio(
360-
convertRawProp(context, rawProps, "aspectRatio", sourceValue.aspectRatio(), yogaStyle.aspectRatio()));
359+
{
360+
const auto *rawValue = rawProps.at("aspectRatio", nullptr, nullptr);
361+
if (rawValue != nullptr) {
362+
yogaStyle.setAspectRatio(rawValue->hasValue() ? convertAspectRatio(context, *rawValue) : yogaStyle.aspectRatio());
363+
} else {
364+
yogaStyle.setAspectRatio(sourceValue.aspectRatio());
365+
}
366+
}
361367

362368
yogaStyle.setBoxSizing(
363369
convertRawProp(context, rawProps, "boxSizing", sourceValue.boxSizing(), yogaStyle.boxSizing()));

packages/react-native/ReactCommon/react/renderer/components/view/tests/ConversionsTest.cpp

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,4 +480,64 @@ TEST(ConversionsTest, unprocessed_font_variant_string_invalid) {
480480
EXPECT_EQ((int)result, (int)FontVariant::Default);
481481
}
482482

483+
TEST(ConversionsTest, convert_aspect_ratio_float) {
484+
RawValue value{folly::dynamic(1.5)};
485+
auto result =
486+
convertAspectRatio(PropsParserContext{-1, ContextContainer{}}, value);
487+
488+
EXPECT_FALSE(result.isUndefined());
489+
EXPECT_EQ(result.unwrap(), 1.5f);
490+
}
491+
492+
TEST(ConversionsTest, convert_aspect_ratio_ratio_string) {
493+
// CSSRatio parses "16/9" as {numerator: 16, denominator: 9}
494+
auto ratio = parseCSSProperty<CSSRatio>("16/9");
495+
ASSERT_TRUE(std::holds_alternative<CSSRatio>(ratio));
496+
auto r = std::get<CSSRatio>(ratio);
497+
EXPECT_FALSE(r.isDegenerate());
498+
EXPECT_NEAR(r.numerator / r.denominator, 16.0f / 9.0f, 0.001f);
499+
}
500+
501+
TEST(ConversionsTest, convert_aspect_ratio_number_string) {
502+
// CSSRatio parses "1.5" as {numerator: 1.5, denominator: 1.0}
503+
auto ratio = parseCSSProperty<CSSRatio>("1.5");
504+
ASSERT_TRUE(std::holds_alternative<CSSRatio>(ratio));
505+
auto r = std::get<CSSRatio>(ratio);
506+
EXPECT_FALSE(r.isDegenerate());
507+
EXPECT_EQ(r.numerator / r.denominator, 1.5f);
508+
}
509+
510+
TEST(ConversionsTest, convert_aspect_ratio_degenerate) {
511+
auto ratio = parseCSSProperty<CSSRatio>("0/0");
512+
ASSERT_TRUE(std::holds_alternative<CSSRatio>(ratio));
513+
EXPECT_TRUE(std::get<CSSRatio>(ratio).isDegenerate());
514+
}
515+
516+
TEST(ConversionsTest, float_optional_from_rawvalue_float) {
517+
RawValue value{folly::dynamic(1.5)};
518+
yoga::FloatOptional result;
519+
fromRawValue(PropsParserContext{-1, ContextContainer{}}, value, result);
520+
521+
EXPECT_FALSE(result.isUndefined());
522+
EXPECT_EQ(result.unwrap(), 1.5f);
523+
}
524+
525+
TEST(ConversionsTest, float_optional_undefined_for_non_float) {
526+
RawValue value{folly::dynamic(nullptr)};
527+
yoga::FloatOptional result;
528+
fromRawValue(PropsParserContext{-1, ContextContainer{}}, value, result);
529+
530+
EXPECT_TRUE(result.isUndefined());
531+
}
532+
533+
TEST(ConversionsTest, float_optional_undefined_for_string) {
534+
// fromRawValue for FloatOptional does not parse strings —
535+
// that is handled by convertAspectRatio specifically.
536+
RawValue value{folly::dynamic("16/9")};
537+
yoga::FloatOptional result;
538+
fromRawValue(PropsParserContext{-1, ContextContainer{}}, value, result);
539+
540+
EXPECT_TRUE(result.isUndefined());
541+
}
542+
483543
} // namespace facebook::react

packages/react-native/ReactCommon/react/renderer/components/view/tests/ViewTest.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
#include <react/renderer/element/Element.h>
2121
#include <react/renderer/element/testUtils.h>
22+
#include <yoga/numeric/FloatOptional.h>
2223

2324
namespace facebook::react {
2425

@@ -229,4 +230,37 @@ TEST_F(YogaDirtyFlagTest, updatingStateForScrollViewMistNotDirtyYogaNode) {
229230
static_cast<RootShadowNode&>(*newRootShadowNode).layoutIfNeeded());
230231
}
231232

233+
TEST_F(YogaDirtyFlagTest, clonedPropsPreserveAspectRatio) {
234+
ContextContainer contextContainer{};
235+
PropsParserContext parserContext{-1, contextContainer};
236+
237+
/*
238+
* Cloning props with empty RawProps must preserve aspectRatio set on the
239+
* source props.
240+
*/
241+
auto newRootShadowNode = rootShadowNode_->cloneTree(
242+
innerShadowNode_->getFamily(), [&](const ShadowNode& oldShadowNode) {
243+
// First clone: set aspectRatio to 1.5
244+
auto viewProps = std::make_shared<ViewShadowNodeProps>();
245+
viewProps->yogaStyle.setAspectRatio(yoga::FloatOptional(1.5f));
246+
auto nodeWithAspectRatio =
247+
oldShadowNode.clone(ShadowNodeFragment{.props = viewProps});
248+
249+
// Second clone: clone props with empty RawProps (simulating a prop
250+
// update that does not touch aspectRatio)
251+
auto& componentDescriptor =
252+
nodeWithAspectRatio->getComponentDescriptor();
253+
auto clonedProps = componentDescriptor.cloneProps(
254+
parserContext, nodeWithAspectRatio->getProps(), RawProps());
255+
256+
auto& clonedViewProps =
257+
static_cast<const ViewShadowNodeProps&>(*clonedProps);
258+
EXPECT_TRUE(clonedViewProps.yogaStyle.aspectRatio().isDefined());
259+
EXPECT_EQ(clonedViewProps.yogaStyle.aspectRatio().unwrap(), 1.5f);
260+
261+
return nodeWithAspectRatio->clone(
262+
ShadowNodeFragment{.props = clonedProps});
263+
});
264+
}
265+
232266
} // namespace facebook::react

0 commit comments

Comments
 (0)