microsoft / react-native-windows

A framework for building native Windows apps with React.
https://microsoft.github.io/react-native-windows/
Other
16.36k stars 1.14k forks source link

New text.windows.js code calls `flatten` on styles twice if not bridgeless #14038

Closed slswalker closed 3 weeks ago

slswalker commented 3 weeks ago

Problem Description

StyleSheet.flatten is very expensive. Running this twice on a component that can be rendered quite often without memoization is quite the perf hit. The second location doesn't look like it was ever tested because it flattens the original style, then grabs a lot of props out of that style, then does not do anything with the result. Please fix this before 0.76 stable release.

Steps To Reproduce

text.windows.js:333

Expected Results

No response

CLI version

npx @react-native-community/cli -v

Environment

npx @react-native-community/cli info

Community Modules

No response

Target Platform Version

None

Target Device(s)

No response

Visual Studio Version

None

Build Configuration

None

Snack, code example, screenshot, or link to a repository

No response

chrisglein commented 3 weeks ago

Adding link to code: https://github.com/microsoft/react-native-windows/blob/ecd493f69825969b143f439d2caa4ee05b90bb28/vnext/src-win/Libraries/Text/Text.windows.js#L333C13-L333C25

Looks like it was modified last month here: https://github.com/microsoft/react-native-windows/pull/13647/files @TatianaKapos do you know if there is anything tracking this change upstream?

Of note this is in Paper. It's entirely possible Meta refactored this upstream and since they're focused on Fabric the behavior on Paper wasn't optimized.

There's a bunch of work to generate a flattened textStyleProps, but then you'll note it isn't used here:

        return (
          <View style={styleProps}>
            <TextAncestor.Provider value={true}>
              {nativeText}
            </TextAncestor.Provider>
          </View>
        );

So... safe to remove all of that work? Or was there something upstream we missed in https://github.com/microsoft/react-native-windows/pull/13647?