search

lugg / react-native-config

Bring some 12 factor love to your mobile apps!
MIT License
4.82k stars 657 forks source link

ReactNativeConfig.h name collision w/ React Native 0.68 iOS #667

Closed jeremy303 closed 1 year ago

jeremy303 commented 2 years ago

React Native 0.68 iOS introduces <react/config/ReactNativeConfig.h> in AppDelegate.mm as part of optional new-architecture support, conflicting with react-native-config's ReactNativeConfig.h.

It seems the docs should be updated to make it clear these are different classes and provide an example of how to work around the name collision.

Thanks!

romainbriand commented 2 years ago

@HolySamosa have you found any solution?

Miyagee commented 2 years ago

Having the same issue. Seems like either this library have to rename the class or RN have to rename the class 🤔

romainbriand commented 2 years ago

Having the same issue. Seems like either this library have to rename the class or RN have to rename the class 🤔

Unfortunately, I had to disable the "native" usage of this lib and hard-coded some values in the native part of app (for our iOS Widget) 😢

Not an expert on this topic but I think the problem comes from this mechanism:

Name conflicts between dynamic shared libraries are not discovered at compile time, link time, or runtime. The dlsym function uses string matching to find symbols. If two libraries use the same name for a function, the dynamic loader returns the first one that matches the symbol name given to dlsym.

https://developer.apple.com/library/archive/documentation/DeveloperTools/Conceptual/DynamicLibraries/100-Articles/DynamicLibraryUsageGuidelines.html#//apple_ref/doc/uid/TP40001928-SW9

Without being 100% sure, I guess the author here should probably "alias" the class. I guess the package is not named properly since the origin:

A good way to solve collision problems in Objective-C is adopt a prefix to differentiate your class name from other class names, and this is the technique already used by Apple itself, lets think about the array class, it is called NSArray [...] https://stackoverflow.com/a/12880582

Or maybe move the library to Swift, which supports namespacing :)

I wish I had skills to offer a fix!

sagark1510 commented 2 years ago

@pedro @luancurti any solution for this. I can't build my app due this on 0.69.1

zrg-team commented 2 years ago

the error still there

exs6350 commented 2 years ago

There is no fix unless you fork this and rename. The author will need to rename the package/podspec because of the name clash with newer versions of react which introduced a "ReactNativeConfig" package in 0.68 and higher.

luancurti commented 1 year ago

Feel free to make a pull request @exs6350 with this solution

philpettican118 commented 1 year ago

Hi @luancurti , if we need to rename the package/podspec to fix the issue, do you have any preferences what the new package should be called?

jeremy303 commented 1 year ago

Sorry, I thought I had long ago replied with my work around. In my case, I've opted out of the "new architecture" for the time being, and I commented out all code in the RCT_NEW_ARCH_ENABLED code blocks.

Also, regarding preferences for the new name, might I suggest TheRealReactNativeConfig. ;-)

jeremy303 commented 1 year ago

My poor attempt at humor aside, Apple recommends using 3 letter prefixes for class names.

So RNC as a prefix seems to be a natural choice.

@philpettican118 @luancurti

philpettican118 commented 1 year ago

@pedro @luancurti please see PR changes. I added an example 0.68.5 project for Android and iOS which seem to work as expected. I don't have access to windows though so I haven't added an example project or been able to test on windows.

jeremy303 commented 1 year ago

This is so awesome. Thanks @philpettican118, for your great work! 🍺