diff --git a/Plugins/BridgeJS/Sources/BridgeJSLink/JSGlueGen.swift b/Plugins/BridgeJS/Sources/BridgeJSLink/JSGlueGen.swift index 51ef16b20..5ee86a57e 100644 --- a/Plugins/BridgeJS/Sources/BridgeJSLink/JSGlueGen.swift +++ b/Plugins/BridgeJS/Sources/BridgeJSLink/JSGlueGen.swift @@ -669,7 +669,7 @@ struct IntrinsicJSFragment: Sendable { } let innerFragment = - if wrappedType.optionalConvention == .stackABI { + if wrappedType.optionalParameterUsesStackABI { try stackLiftFragment(elementType: wrappedType) } else { try liftParameter(type: wrappedType, context: bridgeContext) @@ -686,7 +686,7 @@ struct IntrinsicJSFragment: Sendable { kind: JSOptionalKind, innerFragment: IntrinsicJSFragment ) -> IntrinsicJSFragment { - let isStackConvention = wrappedType.optionalConvention == .stackABI + let isStackConvention = wrappedType.optionalParameterUsesStackABI let absenceLiteral = kind.absenceLiteral let outerParams: [String] @@ -762,7 +762,7 @@ struct IntrinsicJSFragment: Sendable { } let innerFragment = - if wrappedType.optionalConvention == .stackABI { + if wrappedType.optionalParameterUsesStackABI { try stackLowerFragment(elementType: wrappedType) } else { try lowerParameter(type: wrappedType) @@ -779,7 +779,7 @@ struct IntrinsicJSFragment: Sendable { kind: JSOptionalKind, innerFragment: IntrinsicJSFragment ) throws -> IntrinsicJSFragment { - let isStackConvention = wrappedType.optionalConvention == .stackABI + let isStackConvention = wrappedType.optionalParameterUsesStackABI return IntrinsicJSFragment( parameters: ["value"], @@ -2696,6 +2696,24 @@ private extension BridgeType { } } + /// Whether an optional of this type pushes its payload onto the bridge stack + /// when passed as a *parameter*. + /// + /// This usually matches `optionalConvention == .stackABI`, but `jsObject` + /// optionals are the exception: their return values travel through the stack + /// while their parameters use the direct `(isSome, objId)` ABI, matching plain + /// `Optional` and exported `@JS class` parameters. + var optionalParameterUsesStackABI: Bool { + switch self { + case .jsObject: + return false + case .nullable(let wrapped, _): + return wrapped.optionalParameterUsesStackABI + default: + return optionalConvention == .stackABI + } + } + var nilSentinel: NilSentinel { switch self { case .swiftProtocol: diff --git a/Plugins/BridgeJS/Tests/BridgeJSToolTests/Inputs/MacroSwift/Optionals.swift b/Plugins/BridgeJS/Tests/BridgeJSToolTests/Inputs/MacroSwift/Optionals.swift index 5df48d9c0..ea37f5740 100644 --- a/Plugins/BridgeJS/Tests/BridgeJSToolTests/Inputs/MacroSwift/Optionals.swift +++ b/Plugins/BridgeJS/Tests/BridgeJSToolTests/Inputs/MacroSwift/Optionals.swift @@ -30,6 +30,13 @@ class OptionalPropertyHolder { @JS func testOptionalPropertyRoundtrip(_ holder: OptionalPropertyHolder?) -> OptionalPropertyHolder? +// Exported functions taking an optional jsObject use the direct (isSome, objId) +// parameter ABI; the return value travels through 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..65380d1e3 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(_ valueIsSome: Int32, _ valueValue: Int32) -> Void { + #if arch(wasm32) + let ret = roundTripExportedOptionalJSObject(value: Optional.bridgeJSLiftParameter(valueIsSome, valueValue)) + return ret.bridgeJSLowerReturn() + #else + fatalError("Only available on WebAssembly") + #endif +} + +@_expose(wasm, "bjs_roundTripExportedOptionalJSClass") +@_cdecl("bjs_roundTripExportedOptionalJSClass") +public func _bjs_roundTripExportedOptionalJSClass(_ valueIsSome: Int32, _ valueValue: Int32) -> Void { + #if arch(wasm32) + let ret = roundTripExportedOptionalJSClass(value: Optional.bridgeJSLiftParameter(valueIsSome, valueValue)) + 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..d084c8fcf 100644 --- a/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/Optionals.js +++ b/Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/Optionals.js @@ -387,18 +387,9 @@ export async function createInstantiator(options, swift) { setException(error); } } - TestModule["bjs_WithOptionalJSClass_childOrNull_set"] = function bjs_WithOptionalJSClass_childOrNull_set(self, newValue) { + TestModule["bjs_WithOptionalJSClass_childOrNull_set"] = function bjs_WithOptionalJSClass_childOrNull_set(self, newValueIsSome, newValueObjectId) { try { - let optResult; - if (newValue) { - const objId = i32Stack.pop(); - const obj = swift.memory.getObject(objId); - swift.memory.release(objId); - optResult = obj; - } else { - optResult = null; - } - swift.memory.getObject(self).childOrNull = optResult; + swift.memory.getObject(self).childOrNull = newValueIsSome ? swift.memory.getObject(newValueObjectId) : null; } catch (error) { setException(error); } @@ -489,22 +480,13 @@ export async function createInstantiator(options, swift) { setException(error); } } - TestModule["bjs_WithOptionalJSClass_roundTripChildOrNull"] = function bjs_WithOptionalJSClass_roundTripChildOrNull(self, value) { + TestModule["bjs_WithOptionalJSClass_roundTripChildOrNull"] = function bjs_WithOptionalJSClass_roundTripChildOrNull(self, valueIsSome, valueObjectId) { try { - let optResult; - if (value) { - const objId = i32Stack.pop(); - const obj = swift.memory.getObject(objId); - swift.memory.release(objId); - optResult = obj; - } else { - optResult = null; - } - let ret = swift.memory.getObject(self).roundTripChildOrNull(optResult); + let ret = swift.memory.getObject(self).roundTripChildOrNull(valueIsSome ? swift.memory.getObject(valueObjectId) : null); const isSome = ret != null; if (isSome) { - const objId1 = swift.memory.retain(ret); - i32Stack.push(objId1); + const objId = swift.memory.retain(ret); + i32Stack.push(objId); } i32Stack.push(isSome ? 1 : 0); } catch (error) { @@ -725,6 +707,48 @@ 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; + let result; + if (isSome) { + result = swift.memory.retain(value); + } else { + result = 0; + } + instance.exports.bjs_roundTripExportedOptionalJSObject(+isSome, result); + const isSome1 = i32Stack.pop(); + let optResult; + if (isSome1) { + const objId = i32Stack.pop(); + const obj = swift.memory.getObject(objId); + swift.memory.release(objId); + optResult = obj; + } else { + optResult = null; + } + return optResult; + }, + roundTripExportedOptionalJSClass: function bjs_roundTripExportedOptionalJSClass(value) { + const isSome = value != null; + let result; + if (isSome) { + result = swift.memory.retain(value); + } else { + result = 0; + } + instance.exports.bjs_roundTripExportedOptionalJSClass(+isSome, result); + const isSome1 = i32Stack.pop(); + let optResult; + if (isSome1) { + const objId = i32Stack.pop(); + const obj = swift.memory.getObject(objId); + swift.memory.release(objId); + 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..955f31ea9 100644 --- a/Sources/JavaScriptKit/BridgeJSIntrinsics.swift +++ b/Sources/JavaScriptKit/BridgeJSIntrinsics.swift @@ -1826,6 +1826,29 @@ extension _BridgedAsOptional where Wrapped == JSObject { } } +extension _BridgedAsOptional where Wrapped: _JSBridgedClass { + // `@JSClass` wrappers (`_JSBridgedClass`) bridge an underlying `JSObject`, so an + // optional wrapper mirrors `Optional`: parameters use the direct + // (`isSome`, object id) ABI while returns travel through the bridge stack. + // + // Stack push/pop is provided by the generic `Wrapped: _BridgedSwiftStackType` + // extension; only the direct parameter lift and the export return lowering need + // dedicated implementations here. + @_spi(BridgeJS) public static func bridgeJSLiftParameter(_ isSome: Int32, _ objectId: Int32) -> Self { + Self( + optional: Optional._bridgeJSLiftParameter( + isSome, + objectId, + liftWrapped: Wrapped.bridgeJSLiftParameter + ) + ) + } + + @_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..af8308c88 100644 --- a/Tests/BridgeJSRuntimeTests/Generated/BridgeJS.swift +++ b/Tests/BridgeJSRuntimeTests/Generated/BridgeJS.swift @@ -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(_ vIsSome: Int32, _ vValue: Int32) -> Void { + #if arch(wasm32) + let ret = roundTripOptionalImportedClass(v: Optional.bridgeJSLiftParameter(vIsSome, vValue)) + return ret.bridgeJSLowerReturn() + #else + fatalError("Only available on WebAssembly") + #endif +} + @_expose(wasm, "bjs_throwsSwiftError") @_cdecl("bjs_throwsSwiftError") public func _bjs_throwsSwiftError(_ shouldThrow: Int32) -> Void { @@ -14076,6 +14087,18 @@ fileprivate func bjs_OptionalSupportImports_jsRoundTripOptionalStringToStringDic return bjs_OptionalSupportImports_jsRoundTripOptionalStringToStringDictionaryUndefined_static_extern(v) } +#if arch(wasm32) +@_extern(wasm, module: "BridgeJSRuntimeTests", name: "bjs_OptionalSupportImports_jsRoundTripOptionalJSObjectNull_static") +fileprivate func bjs_OptionalSupportImports_jsRoundTripOptionalJSObjectNull_static_extern(_ valueIsSome: Int32, _ valueValue: Int32) -> Void +#else +fileprivate func bjs_OptionalSupportImports_jsRoundTripOptionalJSObjectNull_static_extern(_ valueIsSome: Int32, _ valueValue: Int32) -> Void { + fatalError("Only available on WebAssembly") +} +#endif +@inline(never) fileprivate func bjs_OptionalSupportImports_jsRoundTripOptionalJSObjectNull_static(_ valueIsSome: Int32, _ valueValue: Int32) -> Void { + return bjs_OptionalSupportImports_jsRoundTripOptionalJSObjectNull_static_extern(valueIsSome, valueValue) +} + #if arch(wasm32) @_extern(wasm, module: "BridgeJSRuntimeTests", name: "bjs_OptionalSupportImports_runJsOptionalSupportTests_static") fileprivate func bjs_OptionalSupportImports_runJsOptionalSupportTests_static_extern() -> Void @@ -14162,6 +14185,15 @@ func _$OptionalSupportImports_jsRoundTripOptionalStringToStringDictionaryUndefin return JSUndefinedOr<[String: String]>.bridgeJSLiftReturn() } +func _$OptionalSupportImports_jsRoundTripOptionalJSObjectNull(_ value: Optional) throws(JSException) -> Optional { + let (valueIsSome, valueValue) = value.bridgeJSLowerParameter() + bjs_OptionalSupportImports_jsRoundTripOptionalJSObjectNull_static(valueIsSome, valueValue) + if let error = _swift_js_take_exception() { + throw error + } + return Optional.bridgeJSLiftReturn() +} + func _$OptionalSupportImports_runJsOptionalSupportTests() throws(JSException) -> Void { bjs_OptionalSupportImports_runJsOptionalSupportTests_static() if let error = _swift_js_take_exception() { diff --git a/Tests/BridgeJSRuntimeTests/Generated/JavaScript/BridgeJS.json b/Tests/BridgeJSRuntimeTests/Generated/JavaScript/BridgeJS.json index 94142f470..4f1038b5e 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" : { @@ -21012,6 +21047,40 @@ } } }, + { + "accessLevel" : "internal", + "effects" : { + "isAsync" : false, + "isStatic" : false, + "isThrows" : true + }, + "name" : "jsRoundTripOptionalJSObjectNull", + "parameters" : [ + { + "name" : "value", + "type" : { + "nullable" : { + "_0" : { + "jsObject" : { + + } + }, + "_1" : "null" + } + } + } + ], + "returnType" : { + "nullable" : { + "_0" : { + "jsObject" : { + + } + }, + "_1" : "null" + } + } + }, { "accessLevel" : "internal", "effects" : { diff --git a/Tests/BridgeJSRuntimeTests/JavaScript/OptionalSupportTests.mjs b/Tests/BridgeJSRuntimeTests/JavaScript/OptionalSupportTests.mjs index 6576876da..7c2b991ae 100644 --- a/Tests/BridgeJSRuntimeTests/JavaScript/OptionalSupportTests.mjs +++ b/Tests/BridgeJSRuntimeTests/JavaScript/OptionalSupportTests.mjs @@ -46,6 +46,9 @@ export function getImports(importsContext) { jsRoundTripOptionalStringToStringDictionaryUndefined: (v) => { return v === undefined ? undefined : v; }, + jsRoundTripOptionalJSObjectNull: (v) => { + return v ?? null; + }, runJsOptionalSupportTests: () => { const exports = importsContext.getExports(); if (!exports) { throw new Error("No exports!?"); } diff --git a/Tests/BridgeJSRuntimeTests/OptionalSupportTests.swift b/Tests/BridgeJSRuntimeTests/OptionalSupportTests.swift index 3b06901db..85eaa04c7 100644 --- a/Tests/BridgeJSRuntimeTests/OptionalSupportTests.swift +++ b/Tests/BridgeJSRuntimeTests/OptionalSupportTests.swift @@ -24,6 +24,8 @@ import JavaScriptEventLoop _ v: JSUndefinedOr<[String: String]> ) throws(JSException) -> JSUndefinedOr<[String: String]> + @JSFunction static func jsRoundTripOptionalJSObjectNull(_ value: JSObject?) throws(JSException) -> JSObject? + @JSFunction static func runJsOptionalSupportTests() throws(JSException) } @@ -84,6 +86,15 @@ final class OptionalSupportTests: XCTestCase { func testRoundTripOptionalStringToStringDictionaryUndefined() throws { try roundTripTest(OptionalSupportImports.jsRoundTripOptionalStringToStringDictionaryUndefined, ["key": "value"]) } + + func testRoundTripOptionalJSObjectNull() throws { + try XCTAssertNil(OptionalSupportImports.jsRoundTripOptionalJSObjectNull(nil)) + + let object = JSObject.global.Object.function!.new() + object.testProp = "hello" + let result = try OptionalSupportImports.jsRoundTripOptionalJSObjectNull(object) + XCTAssertEqual(result?.testProp.string, "hello") + } } @JS enum OptionalSupportExports { 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);