-
-
Notifications
You must be signed in to change notification settings - Fork 57
BridgeJS: Swift closure support #463
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
base: main
Are you sure you want to change the base?
Conversation
bbfaa31 to
3d17fda
Compare
| } | ||
|
|
||
| for signature in closureSignatures.sorted(by: { $0.mangleName < $1.mangleName }) { | ||
| let invokeFuncName = "invoke_js_callback_\(signature.mangleName.lowercased())" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be conservative, it might be better not to force lowercase here to avoid conflicts with names having different casing
| ? "Void" | ||
| : parameters.map { $0.mangleTypeName }.joined(separator: "_") | ||
| let returnPart = returnType.mangleTypeName | ||
| self.mangleName = "\(paramPart)_To_\(returnPart)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we encode the module name into the mangled name to avoid conflicts with the same name types from other modules?
| case .swiftHeapObject(let name): | ||
| return name | ||
| case .optional(let wrapped): | ||
| return "Optional\(wrapped.mangleTypeName)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| return "Optional\(wrapped.mangleTypeName)" | |
| return "Optional<\(wrapped.mangleTypeName)>" |
Let's include angle bracket not to conflict with user defined OptionalFoo nominal types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is used for functions and classes name, so we need to avoid < > as this would be treat as generics - maybe we should settle for something like: return "Optional_\(wrapped.mangleTypeName)"
That would also align with
case .closure(let signature):
return "Closure_\(signature.mangleName)"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I think it still has ambiguousity. How about using Swift's mangling rule here? It's very simple if we just want to mangle it. (demangling might be tricky but)
https://github.com/swiftlang/swift/blob/main/docs/ABI/Mangling.rst#types
|
|
||
| if !closureSignatures.isEmpty { | ||
| printer.nextLine() | ||
| printer.write("bjs[\"release_js_callback\"] = function(id) {") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we already have swift_js_release, which does exactly the same thing
| @_spi(BridgeJS) public final class _JSCallbackOwner { | ||
| public let callbackId: Int32 | ||
| private var isReleased: Bool = false | ||
|
|
||
| public init(callbackId: Int32) { | ||
| self.callbackId = callbackId | ||
| } | ||
|
|
||
| deinit { | ||
| guard !isReleased else { return } | ||
| #if arch(wasm32) | ||
| _swift_js_release_callback(callbackId) | ||
| #endif | ||
| isReleased = true | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we consider replacing this class with JSObject? JS object handle deallocation is a bit tricky (e.g. we need to care about deallocation from other threads than it was created.), so I'd like to centralize the lifetime management in JSObject.
Overview
This PR implements typed closure support in BridgeJS, enabling Swift functions to accept closures as parameters and return closures as values with full type safety.
Example
Swift:
JavaScript:
TypeScript:
Technical Changes
BridgeType.closure(ClosureSignature)_BJS_Closure_*) for each unique closure signature_exportsand_bjsto support thatbridgeJSLowerParameterWithRetainto support optionalSwiftHeapObjectSupported Features
@escapingclosuresTesting
Added comprehensive snapshot and runtime tests for closure parameters, returns, optionals, and Swift heap objects in closures.
Documentation
Extended current documentation with new
Exporting-Swift-Closure.md