From 5dbbd02afb418f7066c91dd6f0324ba54d069487 Mon Sep 17 00:00:00 2001 From: Krzysztof Rodak Date: Wed, 3 Jun 2026 12:56:25 +0200 Subject: [PATCH] BridgeJS: Support optional @JSClass as exported function parameters Optional jsObject was moved to the stack ABI in #738, but the export parameter direction in ExportSwift was never updated and still emitted direct ABI params with `Optional.bridgeJSLiftParameter(isSome, value)`. For `@JSClass` types no such overload exists, so generated thunks failed to compile; for plain `JSObject?` it compiled but mismatched the JS stack-push lowering. Make `.nullable(.jsObject)` export parameters use the stack ABI (no direct ABI params, no-argument `Optional.bridgeJSLiftParameter()`), and add the missing `bridgeJSLowerReturn()` for `Optional<_JSBridgedClass>` so optional `@JSClass` values round-trip as both parameters and return values. Fixes #751 --- .../Sources/BridgeJSCore/ExportSwift.swift | 11 +++ .../Inputs/MacroSwift/Optionals.swift | 6 ++ .../BridgeJSCodegenTests/Optionals.json | 70 +++++++++++++++++++ .../BridgeJSCodegenTests/Optionals.swift | 22 ++++++ .../BridgeJSLinkTests/Optionals.d.ts | 2 + .../BridgeJSLinkTests/Optionals.js | 40 +++++++++++ .../JavaScriptKit/BridgeJSIntrinsics.swift | 13 ++++ .../BridgeJSRuntimeTests/ExportAPITests.swift | 4 ++ .../Generated/BridgeJS.swift | 15 +++- .../Generated/JavaScript/BridgeJS.json | 35 ++++++++++ Tests/prelude.mjs | 7 ++ 11 files changed, 223 insertions(+), 2 deletions(-) diff --git a/Plugins/BridgeJS/Sources/BridgeJSCore/ExportSwift.swift b/Plugins/BridgeJS/Sources/BridgeJSCore/ExportSwift.swift index b649b244d..ea2f7f080 100644 --- a/Plugins/BridgeJS/Sources/BridgeJSCore/ExportSwift.swift +++ b/Plugins/BridgeJS/Sources/BridgeJSCore/ExportSwift.swift @@ -1492,6 +1492,11 @@ extension BridgeType { switch self { case .swiftStruct, .array, .dictionary, .associatedValueEnum: return true + case .nullable(.jsObject, _): + // Optional jsObject (incl. @JSClass) parameters travel via the bridge + // stack (isSome + object id), unlike their non-optional counterparts + // which are passed as a direct i32 parameter. + return true case .nullable(let wrapped, _): return wrapped.isStackUsingParameter default: @@ -1531,6 +1536,12 @@ extension BridgeType { case .unsafePointer: return .unsafePointer case .swiftProtocol: return .jsObject case .void: return .void + case .nullable(.jsObject, _): + // Optional jsObject (incl. @JSClass) uses the stack ABI: the presence + // flag and object id are transferred through the bridge stack rather + // than as direct ABI parameters. The lift expression is the no-argument + // `Optional.bridgeJSLiftParameter()` that pops them off the stack. + return LiftingIntrinsicInfo(parameters: []) case .nullable(let wrappedType, _): let wrappedInfo = try wrappedType.liftParameterInfo() if wrappedInfo.parameters.isEmpty { diff --git a/Plugins/BridgeJS/Tests/BridgeJSToolTests/Inputs/MacroSwift/Optionals.swift b/Plugins/BridgeJS/Tests/BridgeJSToolTests/Inputs/MacroSwift/Optionals.swift index 5df48d9c0..f0cab9372 100644 --- a/Plugins/BridgeJS/Tests/BridgeJSToolTests/Inputs/MacroSwift/Optionals.swift +++ b/Plugins/BridgeJS/Tests/BridgeJSToolTests/Inputs/MacroSwift/Optionals.swift @@ -30,6 +30,12 @@ class OptionalPropertyHolder { @JS func testOptionalPropertyRoundtrip(_ holder: OptionalPropertyHolder?) -> OptionalPropertyHolder? +// Exported functions taking/returning an optional jsObject use the stack ABI. +@JS func roundTripExportedOptionalJSObject(value: JSObject?) -> JSObject? + +// Exported function taking/returning an optional imported @JSClass (issue #751). +@JS func roundTripExportedOptionalJSClass(value: WithOptionalJSClass?) -> WithOptionalJSClass? + @JS func roundTripString(name: String?) -> String? { return name diff --git a/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSCodegenTests/Optionals.json b/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSCodegenTests/Optionals.json index 91291c24e..e9d78cbbc 100644 --- a/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSCodegenTests/Optionals.json +++ b/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSCodegenTests/Optionals.json @@ -239,6 +239,76 @@ } } }, + { + "abiName" : "bjs_roundTripExportedOptionalJSObject", + "effects" : { + "isAsync" : false, + "isStatic" : false, + "isThrows" : false + }, + "name" : "roundTripExportedOptionalJSObject", + "parameters" : [ + { + "label" : "value", + "name" : "value", + "type" : { + "nullable" : { + "_0" : { + "jsObject" : { + + } + }, + "_1" : "null" + } + } + } + ], + "returnType" : { + "nullable" : { + "_0" : { + "jsObject" : { + + } + }, + "_1" : "null" + } + } + }, + { + "abiName" : "bjs_roundTripExportedOptionalJSClass", + "effects" : { + "isAsync" : false, + "isStatic" : false, + "isThrows" : false + }, + "name" : "roundTripExportedOptionalJSClass", + "parameters" : [ + { + "label" : "value", + "name" : "value", + "type" : { + "nullable" : { + "_0" : { + "jsObject" : { + "_0" : "WithOptionalJSClass" + } + }, + "_1" : "null" + } + } + } + ], + "returnType" : { + "nullable" : { + "_0" : { + "jsObject" : { + "_0" : "WithOptionalJSClass" + } + }, + "_1" : "null" + } + } + }, { "abiName" : "bjs_roundTripString", "effects" : { diff --git a/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSCodegenTests/Optionals.swift b/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSCodegenTests/Optionals.swift index 0a2528340..bbe0a35ec 100644 --- a/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSCodegenTests/Optionals.swift +++ b/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSCodegenTests/Optionals.swift @@ -20,6 +20,28 @@ public func _bjs_testOptionalPropertyRoundtrip(_ holderIsSome: Int32, _ holderVa #endif } +@_expose(wasm, "bjs_roundTripExportedOptionalJSObject") +@_cdecl("bjs_roundTripExportedOptionalJSObject") +public func _bjs_roundTripExportedOptionalJSObject() -> Void { + #if arch(wasm32) + let ret = roundTripExportedOptionalJSObject(value: Optional.bridgeJSLiftParameter()) + return ret.bridgeJSLowerReturn() + #else + fatalError("Only available on WebAssembly") + #endif +} + +@_expose(wasm, "bjs_roundTripExportedOptionalJSClass") +@_cdecl("bjs_roundTripExportedOptionalJSClass") +public func _bjs_roundTripExportedOptionalJSClass() -> Void { + #if arch(wasm32) + let ret = roundTripExportedOptionalJSClass(value: Optional.bridgeJSLiftParameter()) + return ret.bridgeJSLowerReturn() + #else + fatalError("Only available on WebAssembly") + #endif +} + @_expose(wasm, "bjs_roundTripString") @_cdecl("bjs_roundTripString") public func _bjs_roundTripString(_ nameIsSome: Int32, _ nameBytes: Int32, _ nameLength: Int32) -> Void { diff --git a/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/Optionals.d.ts b/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/Optionals.d.ts index fb9d68db7..c4a22ac0c 100644 --- a/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/Optionals.d.ts +++ b/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/Optionals.d.ts @@ -50,6 +50,8 @@ export type Exports = { } roundTripOptionalClass(value: Greeter | null): Greeter | null; testOptionalPropertyRoundtrip(holder: OptionalPropertyHolder | null): OptionalPropertyHolder | null; + roundTripExportedOptionalJSObject(value: any | null): any | null; + roundTripExportedOptionalJSClass(value: WithOptionalJSClass | null): WithOptionalJSClass | null; roundTripString(name: string | null): string | null; roundTripInt(value: number | null): number | null; roundTripInt8(value: number | null): number | null; diff --git a/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/Optionals.js b/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/Optionals.js index f376c1b24..574bd6f72 100644 --- a/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/Optionals.js +++ b/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/Optionals.js @@ -725,6 +725,46 @@ export async function createInstantiator(options, swift) { const optResult = pointer === null ? null : OptionalPropertyHolder.__construct(pointer); return optResult; }, + roundTripExportedOptionalJSObject: function bjs_roundTripExportedOptionalJSObject(value) { + const isSome = value != null; + if (isSome) { + const objId = swift.memory.retain(value); + i32Stack.push(objId); + } + i32Stack.push(+isSome); + instance.exports.bjs_roundTripExportedOptionalJSObject(); + const isSome1 = i32Stack.pop(); + let optResult; + if (isSome1) { + const objId1 = i32Stack.pop(); + const obj = swift.memory.getObject(objId1); + swift.memory.release(objId1); + optResult = obj; + } else { + optResult = null; + } + return optResult; + }, + roundTripExportedOptionalJSClass: function bjs_roundTripExportedOptionalJSClass(value) { + const isSome = value != null; + if (isSome) { + const objId = swift.memory.retain(value); + i32Stack.push(objId); + } + i32Stack.push(+isSome); + instance.exports.bjs_roundTripExportedOptionalJSClass(); + const isSome1 = i32Stack.pop(); + let optResult; + if (isSome1) { + const objId1 = i32Stack.pop(); + const obj = swift.memory.getObject(objId1); + swift.memory.release(objId1); + optResult = obj; + } else { + optResult = null; + } + return optResult; + }, roundTripString: function bjs_roundTripString(name) { const isSome = name != null; let result, result1; diff --git a/Sources/JavaScriptKit/BridgeJSIntrinsics.swift b/Sources/JavaScriptKit/BridgeJSIntrinsics.swift index ff586b45b..e9c5399e8 100644 --- a/Sources/JavaScriptKit/BridgeJSIntrinsics.swift +++ b/Sources/JavaScriptKit/BridgeJSIntrinsics.swift @@ -1826,6 +1826,19 @@ extension _BridgedAsOptional where Wrapped == JSObject { } } +extension _BridgedAsOptional where Wrapped: _JSBridgedClass { + // `@JSClass` wrappers (`_JSBridgedClass`) bridge an underlying `JSObject`, so an + // optional wrapper uses the same stack ABI as `Optional`: the presence + // flag and retained object id are transferred through the bridge stack. + // + // Lifting (`bridgeJSLiftParameter()` / `bridgeJSLiftReturn()`) and stack push/pop + // are already provided by the generic `Wrapped: _BridgedSwiftStackType` extension; + // only the export return path needs a dedicated lowering here. + @_spi(BridgeJS) public consuming func bridgeJSLowerReturn() -> Void { + Wrapped.bridgeJSStackPushAsOptional(asOptional) + } +} + extension _BridgedAsOptional where Wrapped: _BridgedSwiftProtocolWrapper { @_spi(BridgeJS) public static func bridgeJSLiftParameter(_ isSome: Int32, _ objectId: Int32) -> Self { Self( diff --git a/Tests/BridgeJSRuntimeTests/ExportAPITests.swift b/Tests/BridgeJSRuntimeTests/ExportAPITests.swift index c6e216203..efad25ca1 100644 --- a/Tests/BridgeJSRuntimeTests/ExportAPITests.swift +++ b/Tests/BridgeJSRuntimeTests/ExportAPITests.swift @@ -75,6 +75,10 @@ func runJsWorks() -> Void return try Foo(value) } +@JS func roundTripOptionalImportedClass(v: Foo?) -> Foo? { + return v +} + struct TestError: Error { let message: String } diff --git a/Tests/BridgeJSRuntimeTests/Generated/BridgeJS.swift b/Tests/BridgeJSRuntimeTests/Generated/BridgeJS.swift index 3fa4eb9d5..3660367b9 100644 --- a/Tests/BridgeJSRuntimeTests/Generated/BridgeJS.swift +++ b/Tests/BridgeJSRuntimeTests/Generated/BridgeJS.swift @@ -5262,9 +5262,9 @@ public func _bjs_OptionalSupportExports_static_roundTripOptionalAPIOptionalResul @_expose(wasm, "bjs_OptionalSupportExports_static_takeOptionalJSObject") @_cdecl("bjs_OptionalSupportExports_static_takeOptionalJSObject") -public func _bjs_OptionalSupportExports_static_takeOptionalJSObject(_ valueIsSome: Int32, _ valueValue: Int32) -> Void { +public func _bjs_OptionalSupportExports_static_takeOptionalJSObject() -> Void { #if arch(wasm32) - OptionalSupportExports.takeOptionalJSObject(_: Optional.bridgeJSLiftParameter(valueIsSome, valueValue)) + OptionalSupportExports.takeOptionalJSObject(_: Optional.bridgeJSLiftParameter()) #else fatalError("Only available on WebAssembly") #endif @@ -6940,6 +6940,17 @@ public func _bjs_makeImportedFoo(_ valueBytes: Int32, _ valueLength: Int32) -> I #endif } +@_expose(wasm, "bjs_roundTripOptionalImportedClass") +@_cdecl("bjs_roundTripOptionalImportedClass") +public func _bjs_roundTripOptionalImportedClass() -> Void { + #if arch(wasm32) + let ret = roundTripOptionalImportedClass(v: Optional.bridgeJSLiftParameter()) + return ret.bridgeJSLowerReturn() + #else + fatalError("Only available on WebAssembly") + #endif +} + @_expose(wasm, "bjs_throwsSwiftError") @_cdecl("bjs_throwsSwiftError") public func _bjs_throwsSwiftError(_ shouldThrow: Int32) -> Void { diff --git a/Tests/BridgeJSRuntimeTests/Generated/JavaScript/BridgeJS.json b/Tests/BridgeJSRuntimeTests/Generated/JavaScript/BridgeJS.json index 94142f470..b29321cf1 100644 --- a/Tests/BridgeJSRuntimeTests/Generated/JavaScript/BridgeJS.json +++ b/Tests/BridgeJSRuntimeTests/Generated/JavaScript/BridgeJS.json @@ -11917,6 +11917,41 @@ } } }, + { + "abiName" : "bjs_roundTripOptionalImportedClass", + "effects" : { + "isAsync" : false, + "isStatic" : false, + "isThrows" : false + }, + "name" : "roundTripOptionalImportedClass", + "parameters" : [ + { + "label" : "v", + "name" : "v", + "type" : { + "nullable" : { + "_0" : { + "jsObject" : { + "_0" : "Foo" + } + }, + "_1" : "null" + } + } + } + ], + "returnType" : { + "nullable" : { + "_0" : { + "jsObject" : { + "_0" : "Foo" + } + }, + "_1" : "null" + } + } + }, { "abiName" : "bjs_throwsSwiftError", "effects" : { diff --git a/Tests/prelude.mjs b/Tests/prelude.mjs index 2c922dbe2..5091b1dd6 100644 --- a/Tests/prelude.mjs +++ b/Tests/prelude.mjs @@ -301,6 +301,13 @@ function BridgeJSRuntimeTests_runJsWorks(instance, exports) { assert.ok(foo instanceof ImportedFoo); assert.equal(foo.value, "hello"); + // Optional @JSClass directly as an exported function parameter/return value (issue #751) + const optFoo = new ImportedFoo("optional-foo"); + const optFooResult = exports.roundTripOptionalImportedClass(optFoo); + assert.ok(optFooResult instanceof ImportedFoo); + assert.equal(optFooResult.value, "optional-foo"); + assert.equal(exports.roundTripOptionalImportedClass(null), null); + // Test PropertyHolder with various types const testObj = { testProp: "test" }; const sibling = new exports.SimplePropertyHolder(999);