Skip to content

Commit be4cbdf

Browse files
sammy-SCzoontek
sammy-SC
authored and
zoontek
committed
Move View.js prop transformations to C++ prop parsing (facebook#55853)
Summary: Pull Request resolved: facebook#55853 Changelog: [Internal] `View.js` previously transformed `aria-*` props to `accessibility*` equivalents, `id` to `nativeID`, and `tabIndex` to `focusable` in JavaScript before passing to the native component. This diff moves all those transformations to the C++ prop parsing layer (`AccessibilityProps::setProp`, `Props::setProp`, and `HostPlatformViewProps::setProp`), making the JS wrapper thinner and faster. ## Benchmark Results (View vs ViewNativeComponent) | Component | Before (median ns) | After (median ns) | Change | | View | 666,750 | 546,917 | **-18%** | This change is gated for safe rollout. Reviewed By: javache, NickGerleman Differential Revision: D94219577 fbshipit-source-id: 6dee071a2a8caea533dbedd21a2294aec38307f2
1 parent d74edc9 commit be4cbdf

28 files changed

+1017
-163
lines changed

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

Lines changed: 82 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
import type {ViewProps} from './ViewPropTypes';
1212

13+
import * as ReactNativeFeatureFlags from '../../../src/private/featureflags/ReactNativeFeatureFlags';
1314
import TextAncestorContext from '../../Text/TextAncestorContext';
1415
import ViewNativeComponent from './ViewNativeComponent';
1516
import * as React from 'react';
@@ -28,96 +29,100 @@ component View(
2829
) {
2930
const hasTextAncestor = use(TextAncestorContext);
3031

31-
const {
32-
accessibilityState,
33-
accessibilityValue,
34-
'aria-busy': ariaBusy,
35-
'aria-checked': ariaChecked,
36-
'aria-disabled': ariaDisabled,
37-
'aria-expanded': ariaExpanded,
38-
'aria-hidden': ariaHidden,
39-
'aria-label': ariaLabel,
40-
'aria-labelledby': ariaLabelledBy,
41-
'aria-live': ariaLive,
42-
'aria-selected': ariaSelected,
43-
'aria-valuemax': ariaValueMax,
44-
'aria-valuemin': ariaValueMin,
45-
'aria-valuenow': ariaValueNow,
46-
'aria-valuetext': ariaValueText,
47-
id,
48-
tabIndex,
49-
...otherProps
50-
} = props;
51-
52-
// Since we destructured props, we can now treat it as mutable
53-
const processedProps = otherProps as {...ViewProps};
54-
55-
const parsedAriaLabelledBy = ariaLabelledBy?.split(/\s*,\s*/g);
56-
if (parsedAriaLabelledBy !== undefined) {
57-
processedProps.accessibilityLabelledBy = parsedAriaLabelledBy;
58-
}
32+
let resolvedProps = props;
33+
if (!ReactNativeFeatureFlags.enableNativeViewPropTransformations()) {
34+
const {
35+
accessibilityState,
36+
accessibilityValue,
37+
'aria-busy': ariaBusy,
38+
'aria-checked': ariaChecked,
39+
'aria-disabled': ariaDisabled,
40+
'aria-expanded': ariaExpanded,
41+
'aria-hidden': ariaHidden,
42+
'aria-label': ariaLabel,
43+
'aria-labelledby': ariaLabelledBy,
44+
'aria-live': ariaLive,
45+
'aria-selected': ariaSelected,
46+
'aria-valuemax': ariaValueMax,
47+
'aria-valuemin': ariaValueMin,
48+
'aria-valuenow': ariaValueNow,
49+
'aria-valuetext': ariaValueText,
50+
id,
51+
tabIndex,
52+
...otherProps
53+
} = props;
54+
55+
const processedProps = otherProps as {...ViewProps};
56+
57+
const parsedAriaLabelledBy = ariaLabelledBy?.split(/\s*,\s*/g);
58+
if (parsedAriaLabelledBy !== undefined) {
59+
processedProps.accessibilityLabelledBy = parsedAriaLabelledBy;
60+
}
5961

60-
if (ariaLabel !== undefined) {
61-
processedProps.accessibilityLabel = ariaLabel;
62-
}
62+
if (ariaLabel !== undefined) {
63+
processedProps.accessibilityLabel = ariaLabel;
64+
}
6365

64-
if (ariaLive !== undefined) {
65-
processedProps.accessibilityLiveRegion =
66-
ariaLive === 'off' ? 'none' : ariaLive;
67-
}
66+
if (ariaLive !== undefined) {
67+
processedProps.accessibilityLiveRegion =
68+
ariaLive === 'off' ? 'none' : ariaLive;
69+
}
6870

69-
if (ariaHidden !== undefined) {
70-
processedProps.accessibilityElementsHidden = ariaHidden;
71-
if (ariaHidden === true) {
72-
processedProps.importantForAccessibility = 'no-hide-descendants';
71+
if (ariaHidden !== undefined) {
72+
processedProps.accessibilityElementsHidden = ariaHidden;
73+
if (ariaHidden === true) {
74+
processedProps.importantForAccessibility = 'no-hide-descendants';
75+
}
7376
}
74-
}
7577

76-
if (id !== undefined) {
77-
processedProps.nativeID = id;
78-
}
78+
if (id !== undefined) {
79+
processedProps.nativeID = id;
80+
}
7981

80-
if (tabIndex !== undefined) {
81-
processedProps.focusable = !tabIndex;
82-
}
82+
if (tabIndex !== undefined) {
83+
processedProps.focusable = !tabIndex;
84+
}
8385

84-
if (
85-
accessibilityState != null ||
86-
ariaBusy != null ||
87-
ariaChecked != null ||
88-
ariaDisabled != null ||
89-
ariaExpanded != null ||
90-
ariaSelected != null
91-
) {
92-
processedProps.accessibilityState = {
93-
busy: ariaBusy ?? accessibilityState?.busy,
94-
checked: ariaChecked ?? accessibilityState?.checked,
95-
disabled: ariaDisabled ?? accessibilityState?.disabled,
96-
expanded: ariaExpanded ?? accessibilityState?.expanded,
97-
selected: ariaSelected ?? accessibilityState?.selected,
98-
};
99-
}
86+
if (
87+
accessibilityState != null ||
88+
ariaBusy != null ||
89+
ariaChecked != null ||
90+
ariaDisabled != null ||
91+
ariaExpanded != null ||
92+
ariaSelected != null
93+
) {
94+
processedProps.accessibilityState = {
95+
busy: ariaBusy ?? accessibilityState?.busy,
96+
checked: ariaChecked ?? accessibilityState?.checked,
97+
disabled: ariaDisabled ?? accessibilityState?.disabled,
98+
expanded: ariaExpanded ?? accessibilityState?.expanded,
99+
selected: ariaSelected ?? accessibilityState?.selected,
100+
};
101+
}
102+
103+
if (
104+
accessibilityValue != null ||
105+
ariaValueMax != null ||
106+
ariaValueMin != null ||
107+
ariaValueNow != null ||
108+
ariaValueText != null
109+
) {
110+
processedProps.accessibilityValue = {
111+
max: ariaValueMax ?? accessibilityValue?.max,
112+
min: ariaValueMin ?? accessibilityValue?.min,
113+
now: ariaValueNow ?? accessibilityValue?.now,
114+
text: ariaValueText ?? accessibilityValue?.text,
115+
};
116+
}
100117

101-
if (
102-
accessibilityValue != null ||
103-
ariaValueMax != null ||
104-
ariaValueMin != null ||
105-
ariaValueNow != null ||
106-
ariaValueText != null
107-
) {
108-
processedProps.accessibilityValue = {
109-
max: ariaValueMax ?? accessibilityValue?.max,
110-
min: ariaValueMin ?? accessibilityValue?.min,
111-
now: ariaValueNow ?? accessibilityValue?.now,
112-
text: ariaValueText ?? accessibilityValue?.text,
113-
};
118+
resolvedProps = processedProps;
114119
}
115120

116121
const actualView =
117122
ref == null ? (
118-
<ViewNativeComponent {...processedProps} />
123+
<ViewNativeComponent {...resolvedProps} />
119124
) : (
120-
<ViewNativeComponent {...processedProps} ref={ref} />
125+
<ViewNativeComponent {...resolvedProps} ref={ref} />
121126
);
122127

123128
if (hasTextAncestor) {

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

Lines changed: 135 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* LICENSE file in the root directory of this source tree.
66
*
77
* @flow strict-local
8-
* @fantom_flags enableNativeCSSParsing:*
8+
* @fantom_flags enableNativeCSSParsing:* enableNativeViewPropTransformations:*
99
* @format
1010
*/
1111

@@ -578,6 +578,140 @@ describe('<View>', () => {
578578
});
579579
});
580580

581+
describe('overlapping aria-label and accessibilityLabel', () => {
582+
it('preserves accessibilityLabel when aria-label is removed', () => {
583+
const root = Fantom.createRoot();
584+
585+
// Set both aria-label and accessibilityLabel
586+
Fantom.runTask(() => {
587+
root.render(
588+
<View
589+
aria-label="aria value"
590+
accessibilityLabel="native value"
591+
accessible={true}
592+
/>,
593+
);
594+
});
595+
596+
// aria-label should take precedence
597+
expect(
598+
root.getRenderedOutput({props: ['accessibilityLabel']}).toJSX(),
599+
).toEqual(<rn-view accessibilityLabel="aria value" />);
600+
601+
// Remove aria-label but keep accessibilityLabel
602+
Fantom.runTask(() => {
603+
root.render(
604+
<View accessibilityLabel="native value" accessible={true} />,
605+
);
606+
});
607+
608+
// accessibilityLabel should still be "native value"
609+
expect(
610+
root.getRenderedOutput({props: ['accessibilityLabel']}).toJSX(),
611+
).toEqual(<rn-view accessibilityLabel="native value" />);
612+
});
613+
});
614+
615+
describe('overlapping aria-hidden and importantForAccessibility', () => {
616+
it('preserves importantForAccessibility when aria-hidden is removed', () => {
617+
const root = Fantom.createRoot();
618+
619+
// Set both aria-hidden and importantForAccessibility
620+
Fantom.runTask(() => {
621+
root.render(
622+
<View
623+
aria-hidden={true}
624+
importantForAccessibility="no-hide-descendants"
625+
accessible={true}
626+
/>,
627+
);
628+
});
629+
630+
expect(
631+
root
632+
.getRenderedOutput({props: ['importantForAccessibility']})
633+
.toJSX(),
634+
).toEqual(
635+
<rn-view importantForAccessibility="no-hide-descendants" />,
636+
);
637+
638+
// Remove aria-hidden but keep importantForAccessibility
639+
Fantom.runTask(() => {
640+
root.render(
641+
<View
642+
importantForAccessibility="no-hide-descendants"
643+
accessible={true}
644+
/>,
645+
);
646+
});
647+
648+
// importantForAccessibility should still be "no-hide-descendants"
649+
expect(
650+
root
651+
.getRenderedOutput({props: ['importantForAccessibility']})
652+
.toJSX(),
653+
).toEqual(
654+
<rn-view importantForAccessibility="no-hide-descendants" />,
655+
);
656+
});
657+
});
658+
659+
describe('aria-hidden={false} with importantForAccessibility', () => {
660+
it('does not overwrite explicit importantForAccessibility', () => {
661+
const root = Fantom.createRoot();
662+
663+
// Set importantForAccessibility="yes" and aria-hidden={false}.
664+
// aria-hidden={false} should NOT reset importantForAccessibility
665+
// to Auto, it should preserve the explicit "yes" value.
666+
Fantom.runTask(() => {
667+
root.render(
668+
<View
669+
importantForAccessibility="yes"
670+
aria-hidden={false}
671+
collapsable={false}
672+
/>,
673+
);
674+
});
675+
676+
expect(
677+
root
678+
.getRenderedOutput({props: ['importantForAccessibility']})
679+
.toJSX(),
680+
).toEqual(<rn-view importantForAccessibility="yes" />);
681+
});
682+
});
683+
684+
describe('overlapping aria-live and accessibilityLiveRegion', () => {
685+
it('preserves accessibilityLiveRegion when aria-live is removed', () => {
686+
const root = Fantom.createRoot();
687+
688+
// Set both aria-live and accessibilityLiveRegion
689+
Fantom.runTask(() => {
690+
root.render(
691+
<View
692+
aria-live="polite"
693+
accessibilityLiveRegion="assertive"
694+
accessible={true}
695+
/>,
696+
);
697+
});
698+
699+
// Remove aria-live but keep accessibilityLiveRegion
700+
Fantom.runTask(() => {
701+
root.render(
702+
<View accessibilityLiveRegion="assertive" accessible={true} />,
703+
);
704+
});
705+
706+
// accessibilityLiveRegion should still be "assertive"
707+
expect(
708+
root
709+
.getRenderedOutput({props: ['accessibilityLiveRegion']})
710+
.toJSX(),
711+
).toEqual(<rn-view accessibilityLiveRegion="assertive" />);
712+
});
713+
});
714+
581715
describe('aria-live', () => {
582716
it('is mapped to accessibilityLiveRegion', () => {
583717
const root = Fantom.createRoot();

‎packages/react-native/Libraries/NativeComponent/BaseViewConfig.android.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ const validAttributesForNonEventProps = {
214214
renderToHardwareTextureAndroid: true,
215215
testID: true,
216216
nativeID: true,
217+
id: true,
217218
accessibilityLabelledBy: true,
218219
accessibilityLabel: true,
219220
accessibilityHint: true,
@@ -226,6 +227,19 @@ const validAttributesForNonEventProps = {
226227
experimental_accessibilityOrder: true,
227228
importantForAccessibility: true,
228229
screenReaderFocusable: true,
230+
'aria-busy': true,
231+
'aria-checked': true,
232+
'aria-disabled': true,
233+
'aria-expanded': true,
234+
'aria-hidden': true,
235+
'aria-label': true,
236+
'aria-labelledby': true,
237+
'aria-live': true,
238+
'aria-selected': true,
239+
'aria-valuemax': true,
240+
'aria-valuemin': true,
241+
'aria-valuenow': true,
242+
'aria-valuetext': true,
229243
role: true,
230244
rotation: true,
231245
scaleX: true,
@@ -368,6 +382,7 @@ const validAttributesForNonEventProps = {
368382
borderBlockEndColor: colorAttribute,
369383
borderBlockStartColor: colorAttribute,
370384
focusable: true,
385+
tabIndex: true,
371386
backfaceVisibility: true,
372387
} as const;
373388

‎packages/react-native/Libraries/NativeComponent/BaseViewConfig.ios.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,22 @@ const validAttributesForNonEventProps = {
218218
transformOrigin: true,
219219
accessibilityRole: true,
220220
accessibilityState: true,
221+
'aria-busy': true,
222+
'aria-checked': true,
223+
'aria-disabled': true,
224+
'aria-expanded': true,
225+
'aria-hidden': true,
226+
'aria-label': true,
227+
'aria-labelledby': true,
228+
'aria-live': true,
229+
'aria-selected': true,
230+
'aria-valuemax': true,
231+
'aria-valuemin': true,
232+
'aria-valuenow': true,
233+
'aria-valuetext': true,
234+
tabIndex: true,
221235
nativeID: true,
236+
id: true,
222237
pointerEvents: true,
223238
removeClippedSubviews: true,
224239
role: true,

0 commit comments

Comments
 (0)