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 Turbo Modules #813

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

okwasniewski
Copy link

@okwasniewski okwasniewski commented Aug 12, 2024

This PR describes API design that would allow Swift in Turbo Modules.

I've created this proposal together with @cipolleschi

During this effort I've been able to get Swift Turbo modules working (here is a native function call stopped on break point):

image

A rendered version of the proposal can be read here

proposals/0000-swift-turbo-modules.md Outdated Show resolved Hide resolved
proposals/0000-swift-turbo-modules.md Outdated Show resolved Hide resolved
proposals/0000-swift-turbo-modules.md Outdated Show resolved Hide resolved
proposals/0000-swift-turbo-modules.md Outdated Show resolved Hide resolved
proposals/0000-swift-turbo-modules.md Outdated Show resolved Hide resolved
Copy link

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @okwasniewski for putting this together and to start this conversation!

proposals/0000-swift-turbo-modules.md Outdated Show resolved Hide resolved
proposals/0000-swift-turbo-modules.md Outdated Show resolved Hide resolved
proposals/0000-swift-turbo-modules.md Outdated Show resolved Hide resolved
Co-authored-by: Riccardo Cipolleschi <riccardo.cipolleschi@gmail.com>
The con of this approach are:
* The C++ code remains in the public API of the TurboModule
* We are creating a deeper inheritance chain which usually should be avoided.
* We are asking to all the users to inherit from a different base class. This is hard to make backward compatible.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about using the interop between Swift and C++? What is missing here right now when it comes to making modules? And how much work / breaking change would it need for it? I love this work, but going through ObjC is just going to mean every class still needs to deal with some level of ObjC nonsense, like the NSNumber stuff, and it is an additional layer between the code the library author writes and what is happening under the hood.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, what's stopping Swift and C++ interop is the lack of virtual functions support. So this could be viable once Apple adds this feature. Another issue is that the C++ interop doesn't support having C++ in the header files, which is what React Native is doing. Codegen generates Objective-C interface anyways but we need a way of hiding C++ types from Swift and this is where the ModuleProvider comes in.

Copy link

@TheRogue76 TheRogue76 Aug 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2024-08-13 at 13 41 27 Based on https://developer.apple.com/videos/play/wwdc2024/10136/?time=1307 from WWDC24, the first one should not be an issue moving forward when Swift 6 drops. As for the second one I am not sure what the usage is, can you explain that one a bit more?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi there!
There are several concerns here, to be honest.

What @okwasniewski explained is correct. I was in touch with Apple engineer and they mentioned that the inheritance model between Swift and C++ is too different to implement it properly, as of today. There are many cocnerns related to what a virtual class implies in C++ and a protocol in Swift. TL;DR: it will not happen that Swift class can inherit from C++.

On another side, Swift/Cxx interop is too young and unstable to be used concretely, imho.
I'm very scared that if we start introducing it, people that have custom project might incur in weird compilation issues and that maintenance cost will fall on our shoulder.
Instead, objc/swift interop is well documented and supported and, overall, more stable.

I agree that is perhaps non ideal in terms of ergonomics, but we can try to mitigate the problem Codegenerating as much as we can.


## Detailed design

One of the reasons why we can't adopt Swift in TurboModules is the contamination of C++ types ending up in user-space.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While i can understand not wanting to expose internal C++ types to user land, reality is that RN is based on C++, and if something should be exposed and used by library maintainers, then having a clear API that defines public vs internal is all that is required, the language is inconsequential. I think as a library developer, regardless of what we do, we will eventually have to debug or deal with those internals, and this should be fine. Though maybe its an unpopular opinion

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I completely agree with you. But we would like to provide a cleaner and simple experience to library maintainers. Not every library needs to know how react native works under the hoods, no?

We also have an effort internally to try and define the public boundary better, with a clear distinction between what's public and what's private. The project is almost 10 years old in OSS, so work is not trivial and requires a bit of time, but we hope to get there soon!

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.

3 participants