Skip to content
This repository was archived by the owner on Mar 16, 2019. It is now read-only.

[iOS] Update iOS headers breaks on RN 0.40.0 #244

Closed
HofmannZ opened this issue Jan 26, 2017 · 6 comments
Closed

[iOS] Update iOS headers breaks on RN 0.40.0 #244

HofmannZ opened this issue Jan 26, 2017 · 6 comments

Comments

@HofmannZ
Copy link

HofmannZ commented Jan 26, 2017

Hey could you update the ios headers like this repo did.

They changed it in RN 0.40.0 and brakes all packages with iOS headers 😕.
I suggest you branch of since they did not make it backward compatible so you can still keep support for RN < 0.40.0.

For more info check this: https://github.com/facebook/react-native/releases/tag/v0.40.0.

@BretJohnson
Copy link

To fix this and maintain backwards compatibility with RN <= 0.39 without having to branch you can also change your includes like below, using #if __has_include. That's what we did for our React Native libraries and it worked well.

// Support React Native headers both in the React namespace, where they are in RN version 0.40+,
// and no namespace, for older versions of React Native
#if __has_include(<React/RCTBridge.h>)
#import <React/RCTBridge.h>
#else
#import "RCTBridge.h"
#endif

@fmmajd
Copy link

fmmajd commented Jan 26, 2017

@BretJohnson where should we add this?

@BretJohnson
Copy link

BretJohnson commented Jan 26, 2017

See more here: https://github.com/facebook/react-native/releases/tag/v0.40.0.
Basically every react header now has moved into the React namespace. So every #import "RCT..." you have in your .h and .m files should now change to #import <React/RCT...> for RN version 0.40 and above. For RN versions less than 0.40 it should continue to #import "RCT..." the old way. And that #if __has_include(...) check above can be used to support both versions of RN without branching your code. Here's an example change I made earlier to our stuff for this: microsoft/appcenter-sdk-react-native@2164036.

egunsoma added a commit to egunsoma/react-native-fetch-blob that referenced this issue Jan 26, 2017
@egunsoma
Copy link
Contributor

This works for me on local machine, please verify this.

@wkh237
Copy link
Owner

wkh237 commented Jan 27, 2017

@BretJohnson and @egunsoma , thanks for the information and PR, looks like this is a great way to solve #223 👍 The PR already merged will publish a new release today 😄

wkh237 added a commit that referenced this issue Jan 27, 2017
@wkh237
Copy link
Owner

wkh237 commented Jan 27, 2017

Already published to npm, please upgrade the packge to 0.10.2-beta.8 and verify if it works, thanks !

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants