Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#241: change imports for react-native 0.39.x #242

Merged
merged 4 commits into from
Feb 20, 2017
Merged

#241: change imports for react-native 0.39.x #242

merged 4 commits into from
Feb 20, 2017

Conversation

generalpiston
Copy link
Contributor

No description provided.

@alfonsodev
Copy link

You could use this macro to support previous versions.

#if __has_include("RCTConvert.h")
#import "RCTConvert.h"
#else
#import <React/RCTConvert.h>
#endif

from
https://medium.com/@thisismissem/how-to-upgrade-react-native-modules-in-a-backwards-compatible-manner-a5b5c48d590c#.vh7nbtd26

@generalpiston
Copy link
Contributor Author

Let me know if this is sufficient. Sorry for the delay.

@generalpiston
Copy link
Contributor Author

@alfonsodev does the above changes work for you?

@marian-andre
Copy link

+1

@generalpiston
Copy link
Contributor Author

@benvium or @johanneslumpe could someone commit this?

@benvium
Copy link
Collaborator

benvium commented Feb 18, 2017

Looks good to me. I put in the same backwards-compatible fix on react-native-sound. If someone can confirm they've tested this PR on RN < 40 and RN 40+ I'll merge it.

Away from office at the moment so can't test myself.

@generalpiston
Copy link
Contributor Author

@benvium I've tested with clean installs of 0.39.2 and 0.40.0. I did notice an issue with 0.40.0 that was not happening within my project:

In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:12:
In file included from ../react-native/React/Base/RCTBridge.h:13:
/tmp/rntest040/rn/ios/build/Build/Products/Debug-iphonesimulator/include/React/RCTBridgeModule.h:55:11: warning: duplicate protocol definition of 'RCTBridgeModule' is ignored
@protocol RCTBridgeModule <NSObject>
          ^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:9:
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.h:10:
../react-native/React/Base/RCTBridgeModule.h:55:11: note: previous definition is here
@protocol RCTBridgeModule <NSObject>
          ^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:22:
In file included from ../react-native/React/Base/RCTEventDispatcher.h:12:
/tmp/rntest040/rn/ios/build/Build/Products/Debug-iphonesimulator/include/React/RCTBridge.h:65:1: error: duplicate interface definition for class 'RCTBridge'
@interface RCTBridge : NSObject <RCTInvalidating>
^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:12:
../react-native/React/Base/RCTBridge.h:65:12: note: previous definition is here
@interface RCTBridge : NSObject <RCTInvalidating>
           ^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:22:
In file included from ../react-native/React/Base/RCTEventDispatcher.h:12:
/tmp/rntest040/rn/ios/build/Build/Products/Debug-iphonesimulator/include/React/RCTBridge.h:154:55: error: property has a previous declaration
@property (nonatomic, copy, readonly) NSArray<Class> *moduleClasses;
                                                      ^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:12:
../react-native/React/Base/RCTBridge.h:154:55: note: property declared here
@property (nonatomic, copy, readonly) NSArray<Class> *moduleClasses;
                                                      ^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:22:
In file included from ../react-native/React/Base/RCTEventDispatcher.h:12:
/tmp/rntest040/rn/ios/build/Build/Products/Debug-iphonesimulator/include/React/RCTBridge.h:159:48: error: property has a previous declaration
@property (nonatomic, strong, readonly) NSURL *bundleURL;
                                               ^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:12:
../react-native/React/Base/RCTBridge.h:159:48: note: property declared here
@property (nonatomic, strong, readonly) NSURL *bundleURL;
                                               ^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:22:
In file included from ../react-native/React/Base/RCTEventDispatcher.h:12:
/tmp/rntest040/rn/ios/build/Build/Products/Debug-iphonesimulator/include/React/RCTBridge.h:165:37: error: property has a previous declaration
@property (nonatomic, strong) Class executorClass;
                                    ^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:12:
../react-native/React/Base/RCTBridge.h:165:37: note: property declared here
@property (nonatomic, strong) Class executorClass;
                                    ^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:22:
In file included from ../react-native/React/Base/RCTEventDispatcher.h:12:
/tmp/rntest040/rn/ios/build/Build/Products/Debug-iphonesimulator/include/React/RCTBridge.h:170:61: error: property has a previous declaration
@property (nonatomic, weak, readonly) id<RCTBridgeDelegate> delegate;
                                                            ^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:12:
../react-native/React/Base/RCTBridge.h:170:61: note: property declared here
@property (nonatomic, weak, readonly) id<RCTBridgeDelegate> delegate;
                                                            ^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:22:
In file included from ../react-native/React/Base/RCTEventDispatcher.h:12:
/tmp/rntest040/rn/ios/build/Build/Products/Debug-iphonesimulator/include/React/RCTBridge.h:175:53: error: property has a previous declaration
@property (nonatomic, copy, readonly) NSDictionary *launchOptions;
                                                    ^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:12:
../react-native/React/Base/RCTBridge.h:175:53: note: property declared here
@property (nonatomic, copy, readonly) NSDictionary *launchOptions;
                                                    ^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:22:
In file included from ../react-native/React/Base/RCTEventDispatcher.h:12:
/tmp/rntest040/rn/ios/build/Build/Products/Debug-iphonesimulator/include/React/RCTBridge.h:180:56: error: property has a previous declaration
@property (nonatomic, readonly, getter=isLoading) BOOL loading;
                                                       ^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:12:
../react-native/React/Base/RCTBridge.h:180:56: note: property declared here
@property (nonatomic, readonly, getter=isLoading) BOOL loading;
                                                       ^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:22:
In file included from ../react-native/React/Base/RCTEventDispatcher.h:12:
/tmp/rntest040/rn/ios/build/Build/Products/Debug-iphonesimulator/include/React/RCTBridge.h:185:54: error: property has a previous declaration
@property (nonatomic, readonly, getter=isValid) BOOL valid;
                                                     ^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:12:
../react-native/React/Base/RCTBridge.h:185:54: note: property declared here
@property (nonatomic, readonly, getter=isValid) BOOL valid;
                                                     ^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:22:
In file included from ../react-native/React/Base/RCTEventDispatcher.h:12:
/tmp/rntest040/rn/ios/build/Build/Products/Debug-iphonesimulator/include/React/RCTBridge.h:190:63: error: property has a previous declaration
@property (nonatomic, readonly, strong) RCTPerformanceLogger *performanceLogger;
                                                              ^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:12:
../react-native/React/Base/RCTBridge.h:190:63: note: property declared here
@property (nonatomic, readonly, strong) RCTPerformanceLogger *performanceLogger;
                                                              ^
/tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:150:42: warning: incompatible pointer types sending 'NSException *' to parameter of type 'NSError *' [-Wincompatible-pointer-types]
    return [self reject:reject withError:e];
                                         ^
/tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:515:67: note: passing argument to parameter 'error' here
- (void)reject:(RCTPromiseRejectBlock)reject withError:(NSError *)error
                                                                  ^
/tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:366:34: warning: 'sendAppEventWithName:body:' is deprecated: Subclass RCTEventEmitter instead [-Wdeprecated-declarations]
    [self.bridge.eventDispatcher sendAppEventWithName:[NSString stringWithFormat:@"DownloadBegin-%@", jobId]
                                 ^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:22:
../react-native/React/Base/RCTEventDispatcher.h:77:1: note: 'sendAppEventWithName:body:' has been explicitly marked deprecated here
- (void)sendAppEventWithName:(NSString *)name body:(id)body
^
/tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:374:34: warning: 'sendAppEventWithName:body:' is deprecated: S
ubclass RCTEventEmitter instead [-Wdeprecated-declarations]
    [self.bridge.eventDispatcher sendAppEventWithName:[NSString stringWithFormat:@"DownloadProgress-%@", jobId]
                                 ^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:22:
../react-native/React/Base/RCTEventDispatcher.h:77:1: note: 'sendAppEventWithName:body:' has been explicitly marked deprecated here
- (void)sendAppEventWithName:(NSString *)name body:(id)body
^
/tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:430:34: warning: 'sendAppEventWithName:body:' is deprecated: Subclass RCTEventEmitter instead [-Wdeprecated-declarations]
    [self.bridge.eventDispatcher sendAppEventWithName:[NSString stringWithFormat:@"UploadBegin-%@", jobId]
                                 ^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:22:
../react-native/React/Base/RCTEventDispatcher.h:77:1: note: 'sendAppEventWithName:body:' has been explicitly marked deprecated here
- (void)sendAppEventWithName:(NSString *)name body:(id)body
^
/tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:435:34: warning: 'sendAppEventWithName:body:' is deprecated: Subclass RCTEventEmitter instead [-Wdeprecated-declarations]
    [self.bridge.eventDispatcher sendAppEventWithName:[NSString stringWithFormat:@"UploadProgress-%@", jobId]
                                 ^
In file included from /tmp/rntest040/rn/node_modules/react-native-fs/RNFSManager.m:22:
../react-native/React/Base/RCTEventDispatcher.h:77:1: note: 'sendAppEventWithName:body:' has been explicitly marked deprecated here
- (void)sendAppEventWithName:(NSString *)name body:(id)body
^
6 warnings and 9 errors generated.

This seems like a header is being imported twice. Looking at https://github.com/facebook/react-native/blob/v0.40.0/React/Base/RCTEventDispatcher.h#L12 I can see that the file RCTBridge.h is being imported twice from different headers with slightly different names. A safe solution is to simply remove the import at https://github.com/johanneslumpe/react-native-fs/blob/master/RNFSManager.m#L10. I've updated the pull request to reflect that.

@benvium benvium merged commit 55dd2a7 into itinance:master Feb 20, 2017
@benvium
Copy link
Collaborator

benvium commented Feb 20, 2017

Thanks - I tried it here and worked well.

@generalpiston
Copy link
Contributor Author

@benvium thanks man.

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

Successfully merging this pull request may close these issues.

4 participants