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

Swift support in native modules #207

Closed
mhart opened this issue Mar 26, 2015 · 41 comments
Closed

Swift support in native modules #207

mhart opened this issue Mar 26, 2015 · 41 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@mhart
Copy link

mhart commented Mar 26, 2015

(you all knew this was coming 😸 )

The current documentation suggests Swift is not yet supported, but I thought it would be good to have an issue to at least track progress on that, if it is indeed coming?

There are some related issues here, but I don't think any of them tackle Swift support at a high level:

  1. [Discussion] Process for depending on third-party Objective-C/Swift code #68
  2. Adds bridging header helper to facilitate importing to swift #115
  3. How do I run the packager for a new project? #23
@brentvatne
Copy link
Collaborator

@mhart - My understanding is that any Swift support would need to be a community driven effort, given that Facebook uses Objective C internally at the moment. Until this changes there's probably no need to have an issue tracking this, we have enough open issues already 😄

If you'd like to lead this effort it would definitely be welcome! Feel free to create another repo and get in touch in IRC.

@mhart
Copy link
Author

mhart commented Apr 15, 2015

Damn 😿

I'm surprised it's not on your roadmap!

@randomer
Copy link

Same here. Especially considering the fact that in case some not-yet-implemented native functionality is needed, it would be much easier for a JS developer to learn Swift than Objective-C (which at least at the first glance looks like a mess)

@ide
Copy link
Contributor

ide commented Apr 16, 2015

Not to derail the topic at hand but for context: there are lots of little things about Swift that make Objective-C the better choice for now. The tooling for Swift is premature in that build times are worse, every app that uses Swift adds ~6MB to its binary for the runtime, Xcode crashes frequently, the Swift compiler produces code with bugs, the language is changing in backwards-incompatible ways, etc. Apple will fix these issues but it will be a year or two before developing with Swift is better than developing with Objective-C.

2c: my experience learning the languages was that Objective-C was actually easier since it's a relatively thin layer above C and is a rather dynamic language. It also works well with all the iOS APIs whereas sometimes there are interop issues with Swift. So generally speaking, I expect developers to have a better time writing native React modules in Objective-C at this time.

That all said, eventually supporting Swift probably makes sense as it improves and exploring how Swift could export native methods to JS would be useful.

@frankbolviken
Copy link

Dissapointing..

@nicklockwood
Copy link
Contributor

We won't be using Swift internally within the framework for the foreseeable future for all the reasons that @ide mentions, however we would welcome support for writing modules and views in Swift.

The reason this is difficult at the moment is because we're heavily reliant on macros for setting up the bindings between Objective-C code and the JavaScript bridge, and Swift has no support for macros whatsoever.

I'm not particularly a fan of macros in general, but it makes this process simple and elegant to an extent that would be hard (if not impossible) to achieve any other way, and crucially it isolates the syntax from the implementation, giving us freedom to make changes to the infrastructure without constantly breaking everyone's existing code.

To support modules written in 100% swift, we would need to provide a totally separate mechanism for registering the module class, methods and properties because writing the bridging code that the Obj-C macros generate manually in Swift would be cumbersome and fragile (assuming that it's even possible).

In the future, once the APIs settle, we may be able to move to a more traditional protocol-based approach, which would be better suited to Swift. In the meantime though, if you don't mind writing a little bit of glue code in Objective-C, it's perfectly possible to implement the bulk of your native module logic in Swift if you wish to do so.

@robertjpayne
Copy link
Contributor

It is possible to utilise Swift without too much work. Because you solely need to generate some metadata for React Native you can get away with just creating nearly blank categories, though it does require at least 1 Objective-C implementation file per Swift class. ( No header or #import necessary! )

@ide @mhart @brentvatne @randomer @frankbolviken
See my gist here with example how-to:

https://gist.github.com/robertjpayne/855fdb15d5ceca12f6c5

Note that this implementation is pretty fragile and relies on a bit of knowledge about how react-native currently generates it's metadata for the bridging.

@nicklockwood is there any chance you'd be open to maintaining a set of Swift compatible macros? I'm just not sure how much of a moving target you expect the current macros to be.

@mhart
Copy link
Author

mhart commented Apr 22, 2015

@robertjpayne Very cool! Would be great to have a better home for this for people to follow if you do indeed want to keep it up to date (like a dedicated repo). I'm sure that would get quite popular 😄 But totally get that might be too much work if things are shifting rapidly.

I mean, ideally, Facebook would just add this sort of thing to their docs somewhere.

@joewalnes
Copy link

@robertjpayne Nice! 👍

@robertjpayne
Copy link
Contributor

Yea I thought about a repo but given it's only the single header file it seems excessive. I'll wait and see if @nicklockwood chimes in about the moving target.

@mhart
Copy link
Author

mhart commented Apr 22, 2015

@robertjpayne I feel ya – I think the README would be larger than the actual code 😃, but that's what people would be interested in I'd say. I'd be fine with a gist if GitHub actually made them easy to follow and manage comments...

@joewood
Copy link

joewood commented Apr 22, 2015

How about adding this to react-native-cli as a command to add a Swift class to an existing app?

react-native addswift Foo

@nicklockwood nicklockwood reopened this Apr 22, 2015
@nicklockwood
Copy link
Contributor

Reopening this as it seems to be a useful place to track this discussion.

@brentvatne
Copy link
Collaborator

@nicklockwood - sounds good, seemed stagnant for a while but the discussion has really picked up since I closed the issue, go figure! 😄

@nicklockwood
Copy link
Contributor

@robertjpayne awesome work - I appreciate you taking the initiative with this.

I agree that this seems like Swift module support should be part of the core rather than something external because it will be very hard to ensure macros are kept in sync.

Your RCT_SWIFT_EXPORT_MODULE macro can be simplified to:

#define RCT_SWIFT_EXPORT_MODULE_PUSH(module_name, superclass, js_name) \
  @interface module_name : superclass \
  @end \
  @interface module_name (RCT_SWIFT_MODULE_EXPORT) <RCTBridgeModule> \
  @end \
  @implementation module_name (RCT_SWIFT_MODULE_EXPORT) \
  RCT_EXPORT_MODULE(js_name) \

I thought it could be reduced to just

#define RCT_SWIFT_EXPORT_MODULE_PUSH(module_name, superclass, js_name) \
  @interface module_name : superclass <RCTBridgeModule> \
  @end \
  @implementation module_name (RCT_SWIFT_MODULE_EXPORTS)  \
  RCT_EXPORT_MODULE(js_name) \

But it seems that with this approach, module_name isn't registered as conforming to RCTBridgeModule, which is a little strange (though probably easy to fix in the bridge logic itself).

@robertjpayne
Copy link
Contributor

@nicklockwood the first interface is solely there to declare to Objective-C that the class exists since it's implemented in Swift. It's essentially just the public header file.

To add the protocol conformance it has to be done inside the category. Though theoretically the user could add the protocol in Swift.

@nicklockwood
Copy link
Contributor

The RCT_SWIFT_EXPORT_MODULE_PUSH/POP syntax is quite elegant, but it has shades of trying to reinvent the Objective-C language, which we've been trying to avoid (macros are seductive). I'm going to think about this for a bit and see if I can come up with something less obtrusive.

@nicklockwood
Copy link
Contributor

@robertjpayne I tried both those code examples before posting them. It seems to work without the extra category.

@robertjpayne
Copy link
Contributor

@nicklockwood right! I agree about the macros, since there's less interface boilerplate it may not be necessary to have the PUSH/POP but just make the user define the interface, category and then add the export methods inside the category definition.

@nicklockwood
Copy link
Contributor

I was thinking that the RCT_SWIFT_EXPORT_METHOD macro could be called something like RCT_EXTERN_METHOD instead (meaning "this method is declared elsewhere"). Then we could implement RCT_EXPORT_METHOD in terms of RCT_EXTERN_METHOD:

#define RCT_EXPORT_METHOD(method) \
  RCT_EXTERN_METHOD(method)
  - (void)method

That way, we avoid duplicating the macro logic, which should make it easier to maintain.

@ide
Copy link
Contributor

ide commented Apr 22, 2015

Re: class definition - I get the need for the category but think you could get rid of the first @interface declaration and pull in the swift bridging header instead.

@nicklockwood
Copy link
Contributor

@ide but then you'd have to add the class to the bridging header, right? This way, you don't have to, as the Swift class doesn't need to reference any symbols in SwiftReactModuleExports.m

@ide
Copy link
Contributor

ide commented Apr 22, 2015

@nicklockwood oops - I meant generated header instead of the bridging header. It's just #import "App-Swift.h".

@robertjpayne
Copy link
Contributor

@ide I've never had good luck with the generated headers, the tooling for them is buggy, it's much more reliable to generate the a small interface stub to work with.

@ide
Copy link
Contributor

ide commented Apr 22, 2015

@robertjpayne have you had issues with the latest Xcode? Swift got a lot more tenable in the last release and a lot of compiler issues went away.

@robertjpayne
Copy link
Contributor

@ide yup, with my simple example I've tried just now and it refuses to generate a header for the class (Xcode 6.3.1). I think probably because the class is not referenced -anywhere- so the Swift compiler is probably stripping it.

@ide
Copy link
Contributor

ide commented Apr 22, 2015

Boo Apple. OK - the @interface stub sounds good to me.

@robertjpayne
Copy link
Contributor

@nicklockwood I'll work on cleaning this up best I can so we don't use more macro magic than necessary.

It's going to look something like:

@interface MySwiftModule : NSObject <RCTBridgeModule>
@end
@implementation MySwiftModule (RCTBridgeModuleExterns)

RCT_EXTERN_MODULE(MySwiftModule)

RCT_EXTERN_METHOD(printMessage:(NSString *)message)
RCT_EXTERN_REMAP_METHOD(printMessage, printMessage:(NSString *)message)

@end

We could deduce this entirely into just the interface declaration if we're ok asking users to add @objc class func moduleName() -> String { return "MyJSModuleName" } to their Swift files if they want a custom name and creating macros that don't generate function implementations as they're not necessary anyways.

@nicklockwood
Copy link
Contributor

We could derive the js name from the category name, but that might be a little too magical :-)

@nicklockwood
Copy link
Contributor

Another pattern I considered is

RCT_EXTERN_MODULE(MySwiftModule,
  RCT_EXTERN_METHOD(foo);
  RCT_EXTERN_METHOD(Bar);
)

@nicklockwood
Copy link
Contributor

This would have the benefit of generating all the boilerplate for you, and it feels a bit less magical than the push/pop to me. That may be purely subjective on my part though.

What do you think?

@robertjpayne
Copy link
Contributor

Yup I like that idea I'll try and get something working in the next few hours.

On 23/04/2015, at 12:11 pm, Nick Lockwood [email protected] wrote:

This would have the benefit of generating all the boilerplate for you, and it feels a bit less magical than the push/pop to me. That may be purely subjective on my part though.

What do you think?


Reply to this email directly or view it on GitHub.

@robertjpayne
Copy link
Contributor

Only thing is we still have to get the user to specify superclass for the stubbed interface. I can't find a way around that unless I can reliably get generated headers to work.

On 23/04/2015, at 12:11 pm, Nick Lockwood [email protected] wrote:

This would have the benefit of generating all the boilerplate for you, and it feels a bit less magical than the push/pop to me. That may be purely subjective on my part though.

What do you think?


Reply to this email directly or view it on GitHub.

@nicklockwood
Copy link
Contributor

True. Maybe something more like this then?

@interface RCT_EXTERN_MODULE(MyModule, NSObject)

RCT_EXTERN_METHOD(foo);
RCT_EXTERN_METHOD(Bar);

@EnD

@robertjpayne
Copy link
Contributor

@nicklockwood how does this look?

#define RCT_EXTERN_MODULE(objc_name, objc_supername) \
  RCT_EXTERN_REMAP_MODULE(objc_name, objc_name, objc_supername)

#define RCT_EXTERN_REMAP_MODULE(js_name, objc_name, objc_supername) \
  objc_name : objc_supername \
  @end \
  @interface objc_name (RCTExternModule) <RCTBridgeModule> \
  @end \
  @implementation objc_name (RCTExternModule) \
  RCT_EXPORT_MODULE(js_name)

#define RCT_EXTERN_METHOD(method) \
  RCT_EXTERN_REMAP_METHOD(, method)

#define RCT_EXTERN_REMAP_METHOD(js_name, method) \
  - (void)__rct_export__##method { \
  __attribute__((used, section("__DATA,RCTExport"))) \
  __attribute__((__aligned__(1))) \
    static const char *__rct_export_entry__[] = { __func__, #method, #js_name }; \

Allows two ways to make them:

Custom Module/Method Name:

@interface RCT_EXTERN_REMAP_MODULE(AwesomeJS, SwiftReactModule, NSObject)

RCT_EXTERN_REMAP_METHOD(doAwesome, printMessage:(NSString *)message)

@end
var AwesomeJS = require('NativeModules').AwesomeJS;
AwesomeJS.doAwesome("Hello World");

Inferred Module/Method Name:

@interface RCT_EXTERN_MODULE(SwiftReactModule, NSObject)

RCT_EXTERN_METHOD(printMessage:(NSString *)message)

@end
var SwiftReactModule = require('NativeModules').SwiftReactModule;
SwiftReactModule.printMessage("Hello World");

@robertjpayne
Copy link
Contributor

@nicklockwood unfortunately we have to redefine the method macro contents because the current RCT_REMAP_METHOD macro currently starts the concrete method's implementation which, because it's a category, overrides the swift method.

Best option would be to have these extern method macro contain the "header logic" and the non-extern methods macro contain the extern macro + implementation logic.

@nicklockwood
Copy link
Contributor

@robertjpayne yep, this looks good to me. Yeah, I realised we'd have to reimplement RCT_EXPORT_METHOD by having it call RCT_EXTERN_METHOD (see my earlier comment), but I think that's not a problem if RCT_EXTERN_METHOD is going to be part of the core.

@robertjpayne
Copy link
Contributor

@nicklockwood cool, well if you're happy with this I'll go ahead and whip up a PR and remap RCT_EXPORT_METHOD to use RCT_EXTERN_METHOD.

@nicklockwood
Copy link
Contributor

@robertjpayne good call on RCT_EXTERN_REMAP_MODULE as well - I like the consistency there. I'll run this by the rest of the team today, but I'm pretty happy with this solution overall.

@nicklockwood
Copy link
Contributor

@robertjpayne please do :-)

@mhart
Copy link
Author

mhart commented Apr 25, 2015

Nice work ppl! 🎉 So I guess this can be closed now thanks to b72acc2 ?

@vjeux vjeux closed this as completed May 29, 2015
@facebook facebook locked as resolved and limited conversation to collaborators May 29, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests