facebook / react-native

A framework for building native applications using React
https://reactnative.dev
MIT License
116.27k stars 23.99k forks source link

[IOS] Remove redundant fmt dependencies #44449

Open Saadnajmi opened 1 week ago

Saadnajmi commented 1 week ago

Summary:

fmt is required by many different podspecs, but is only used by Folly. I verified this by removing fat altogether and seeing what build errors I got. Cocoapods supports transitive dependencies, so let's simplify the build graph a bit and mark fmt as only a dependency of folly.

Changelog:

[IOS] [CHANGED] - Remove redundant fmt dependencies

Test Plan:

CI should pass. Local build of RNTester works.

analysis-bot commented 1 week ago
Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 19,495,291 +37
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 22,867,570 +23
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: be09d12667044f237f08af410b2838062eb8e657 Branch: main

NickGerleman commented 1 week ago

DoubleConversion is in this same boat, if we are able to make exported headers work nicely everywhere.

Saadnajmi commented 1 week ago

DoubleConversion is in this same boat, if we are able to make exported headers work nicely everywhere.

I can take a look. I was looking at cleaning up the header search paths too

Saadnajmi commented 1 week ago

@NickGerleman It looks I can remove the header search paths for fmt, and the dependency/header search paths for DoubleConversion too. One thing I'm not clear about is that I think codegen (which generates ReactCodegen.podspec.json) needs both, so I've left fat and DoubleConversion a dependency there. I'm not sure if it's fine for codegen to get the dependency through folly, or if it generates code that uses either library.

Saadnajmi commented 1 week ago

@NickGerleman I did some testing. Removing the fmt header search paths breaks dynamic frameworks support, since I guess most things depend on folly eventually. Removing the DoubleConversion dependency seems to be in the same boat. So I think the change in its current form is what works.