Merge pull request #113 from swiftwasm/katei/update-jsclosure

Major API change on JSClosure

Author: kateinoigakukun

.github/workflows/compatibility.yml

@@ -20,5 +20,5 @@ jobs:
           export PATH="$SWIFTENV_ROOT/bin:$PATH"
           eval "$(swiftenv init -)"
           make bootstrap
-          cd Example
+          cd Example/JavaScriptKitExample
           swift build --triple wasm32-unknown-wasi

Example/JavaScriptKitExample/Sources/JavaScriptKitExample/main.swift

@@ -12,6 +12,6 @@ buttonElement.innerText = "Click me!"
 let listener = JSClosure { _ in
     alert("Swift is running on browser!")
 }
-buttonElement.onclick = .function(listener)
+buttonElement.onclick = .object(listener)
 
 _ = document.body.appendChild(buttonElement)

IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift

@@ -183,6 +183,47 @@ try test("Function Call") {
     try expectEqual(func6(true, "OK", 2), .string("OK"))
 }
 
+let evalClosure = JSObject.global.globalObject1.eval_closure.function!
+
+try test("Closure Lifetime") {
+    do {
+        let c1 = JSClosure { arguments in
+            return arguments[0]
+        }
+        try expectEqual(evalClosure(c1, JSValue.number(1.0)), .number(1.0))
+        c1.release()
+    }
+
+    do {
+        let c1 = JSClosure { arguments in
+            return arguments[0]
+        }
+        c1.release()
+        // Call a released closure
+        _ = try expectThrow(try evalClosure.throws(c1))
+    }
+
+    do {
+        let c1 = JSClosure { _ in
+            // JSClosure will be deallocated before `release()`
+            _ = JSClosure { _ in .undefined }
+            return .undefined
+        }
+        _ = try expectThrow(try evalClosure.throws(c1))
+        c1.release()
+    }
+
+    do {
+        let c1 = JSOneshotClosure { _ in
+            return .boolean(true)
+        }
+        try expectEqual(evalClosure(c1), .boolean(true))
+        // second call will cause `fatalError` that can be caught as a JavaScript exception
+        _ = try expectThrow(try evalClosure.throws(c1))
+        // OneshotClosure won't call fatalError even if it's deallocated before `release`
+    }
+}
+
 try test("Host Function Registration") {
     // ```js
     // global.globalObject1 = {
@@ -205,7 +246,7 @@ try test("Host Function Registration") {
         return .number(1)
     }
 
-    setJSValue(this: prop_6Ref, name: "host_func_1", value: .function(hostFunc1))
+    setJSValue(this: prop_6Ref, name: "host_func_1", value: .object(hostFunc1))
 
     let call_host_1 = getJSValue(this: prop_6Ref, name: "call_host_1")
     let call_host_1Func = try expectFunction(call_host_1)
@@ -223,8 +264,8 @@ try test("Host Function Registration") {
         }
     }
 
-    try expectEqual(hostFunc2(3), .number(6))
-    _ = try expectString(hostFunc2(true))
+    try expectEqual(evalClosure(hostFunc2, 3), .number(6))
+    _ = try expectString(evalClosure(hostFunc2, true))
     hostFunc2.release()
 }
 
@@ -336,7 +377,7 @@ try test("ObjectRef Lifetime") {
 
     let identity = JSClosure { $0[0] }
     let ref1 = getJSValue(this: .global, name: "globalObject1").object!
-    let ref2 = identity(ref1).object!
+    let ref2 = evalClosure(identity, ref1).object!
     try expectEqual(ref1.prop_2, .number(2))
     try expectEqual(ref2.prop_2, .number(2))
     identity.release()
@@ -449,21 +490,29 @@ try test("Date") {
 }
 
 // make the timers global to prevent early deallocation
-var timeout: JSTimer?
+var timeouts: [JSTimer] = []
 var interval: JSTimer?
 
 try test("Timer") {
     let start = JSDate().valueOf()
     let timeoutMilliseconds = 5.0
-
+    var timeout: JSTimer!
     timeout = JSTimer(millisecondsDelay: timeoutMilliseconds, isRepeating: false) {
         // verify that at least `timeoutMilliseconds` passed since the `timeout` timer started
         try! expectEqual(start + timeoutMilliseconds <= JSDate().valueOf(), true)
     }
+    timeouts += [timeout]
+
+    timeout = JSTimer(millisecondsDelay: timeoutMilliseconds, isRepeating: false) {
+        fatalError("timer should be cancelled")
+    }
+    timeout = nil
 
     var count = 0.0
     let maxCount = 5.0
     interval = JSTimer(millisecondsDelay: 5, isRepeating: true) {
+        // ensure that JSTimer is living
+        try! expectNotNil(interval)
         // verify that at least `timeoutMilliseconds * count` passed since the `timeout` 
         // timer started
         try! expectEqual(start + timeoutMilliseconds * count <= JSDate().valueOf(), true)

IntegrationTests/bin/primary-tests.js

@@ -47,6 +47,9 @@ global.globalObject1 = {
             throw 3.0
         },
     },
+    eval_closure: function(fn) {
+        return fn(arguments[1])
+    }
 };
 
 global.Animal = function (name, age, isCat) {

Sources/JavaScriptKit/BasicObjects/JSPromise.swift

@@ -8,20 +8,11 @@ is of actual JavaScript `Error` type, you should use `JSPromise<JSValue, JSValue
 This doesn't 100% match the JavaScript API, as `then` overload with two callbacks is not available.
 It's impossible to unify success and failure types from both callbacks in a single returned promise
 without type erasure. You should chain `then` and `catch` in those cases to avoid type erasure.
-
-**IMPORTANT**: instances of this class must have the same lifetime as the actual `Promise` object in
-the JavaScript environment, because callback handlers will be deallocated when `JSPromise.deinit` is
-executed.
-
-If the actual `Promise` object in JavaScript environment lives longer than this `JSPromise`, it may
-attempt to call a deallocated `JSClosure`.
 */
 public final class JSPromise<Success, Failure>: ConvertibleToJSValue, ConstructibleFromJSValue {
     /// The underlying JavaScript `Promise` object.
     public let jsObject: JSObject
 
-    private var callbacks = [JSClosure]()
-
     /// The underlying JavaScript `Promise` object wrapped as `JSValue`.
     public func jsValue() -> JSValue {
         .object(jsObject)
@@ -52,39 +43,37 @@ public final class JSPromise<Success, Failure>: ConvertibleToJSValue, Constructi
     /** Schedules the `success` closure to be invoked on sucessful completion of `self`.
     */
     public func then(success: @escaping () -> ()) {
-        let closure = JSClosure { _ in success() }
-        callbacks.append(closure)
+        let closure = JSOneshotClosure { _ in
+            success()
+            return .undefined
+        }
         _ = jsObject.then!(closure)
     }
 
     /** Schedules the `failure` closure to be invoked on either successful or rejected completion of 
     `self`.
     */
     public func finally(successOrFailure: @escaping () -> ()) -> Self {
-        let closure = JSClosure { _ in
+        let closure = JSOneshotClosure { _ in
             successOrFailure()
+            return .undefined
         }
-        callbacks.append(closure)
         return .init(unsafe: jsObject.finally!(closure).object!)
     }
-
-    deinit {
-        callbacks.forEach { $0.release() }
-    }
 }
 
 extension JSPromise where Success == (), Failure == Never {
     /** Creates a new `JSPromise` instance from a given `resolver` closure. `resolver` takes 
     a closure that your code should call to resolve this `JSPromise` instance.
     */
     public convenience init(resolver: @escaping (@escaping () -> ()) -> ()) {
-        let closure = JSClosure { arguments -> () in
+        let closure = JSOneshotClosure { arguments in
             // The arguments are always coming from the `Promise` constructor, so we should be
             // safe to assume their type here
             resolver { arguments[0].function!() }
+            return .undefined
         }
         self.init(unsafe: JSObject.global.Promise.function!.new(closure))
-        callbacks.append(closure)
     }
 }
 
@@ -93,7 +82,7 @@ extension JSPromise where Failure: ConvertibleToJSValue {
     two closure that your code should call to either resolve or reject this `JSPromise` instance.
     */
     public convenience init(resolver: @escaping (@escaping (Result<Success, JSError>) -> ()) -> ()) {
-        let closure = JSClosure { arguments -> () in
+        let closure = JSOneshotClosure { arguments in
             // The arguments are always coming from the `Promise` constructor, so we should be
             // safe to assume their type here
             let resolve = arguments[0].function!
@@ -107,9 +96,9 @@ extension JSPromise where Failure: ConvertibleToJSValue {
                     reject(error.jsValue())
                 }
             }
+            return .undefined
         }
         self.init(unsafe: JSObject.global.Promise.function!.new(closure))
-        callbacks.append(closure)
     }
 }
 
@@ -118,7 +107,7 @@ extension JSPromise where Success: ConvertibleToJSValue, Failure: JSError {
     a closure that your code should call to either resolve or reject this `JSPromise` instance.
     */
     public convenience init(resolver: @escaping (@escaping (Result<Success, JSError>) -> ()) -> ()) {
-        let closure = JSClosure { arguments -> () in
+        let closure = JSOneshotClosure { arguments in
             // The arguments are always coming from the `Promise` constructor, so we should be
             // safe to assume their type here
             let resolve = arguments[0].function!
@@ -132,9 +121,9 @@ extension JSPromise where Success: ConvertibleToJSValue, Failure: JSError {
                     reject(error.jsValue())
                 }
             }
+            return .undefined
         }
         self.init(unsafe: JSObject.global.Promise.function!.new(closure))
-        callbacks.append(closure)
     }
 }
 
@@ -146,13 +135,13 @@ extension JSPromise where Success: ConstructibleFromJSValue {
         file: StaticString = #file,
         line: Int = #line
     ) {
-        let closure = JSClosure { arguments -> () in
+        let closure = JSOneshotClosure { arguments in
             guard let result = Success.construct(from: arguments[0]) else {
                 fatalError("\(file):\(line): failed to unwrap success value for `then` callback")
             }
             success(result)
+            return .undefined
         }
-        callbacks.append(closure)
         _ = jsObject.then!(closure)
     }
 
@@ -165,13 +154,12 @@ extension JSPromise where Success: ConstructibleFromJSValue {
         file: StaticString = #file,
         line: Int = #line
     ) -> JSPromise<ResultType, Failure> {
-        let closure = JSClosure { arguments -> JSValue in
+        let closure = JSOneshotClosure { arguments -> JSValue in
             guard let result = Success.construct(from: arguments[0]) else {
                 fatalError("\(file):\(line): failed to unwrap success value for `then` callback")
             }
             return success(result).jsValue()
         }
-        callbacks.append(closure)
         return .init(unsafe: jsObject.then!(closure).object!)
     }
 
@@ -184,13 +172,12 @@ extension JSPromise where Success: ConstructibleFromJSValue {
         file: StaticString = #file,
         line: Int = #line
     ) -> JSPromise<ResultSuccess, ResultFailure> {
-        let closure = JSClosure { arguments -> JSValue in
+        let closure = JSOneshotClosure { arguments -> JSValue in
             guard let result = Success.construct(from: arguments[0]) else {
                 fatalError("\(file):\(line): failed to unwrap success value for `then` callback")
             }
             return success(result).jsValue()
         }
-        callbacks.append(closure)
         return .init(unsafe: jsObject.then!(closure).object!)
     }
 }
@@ -205,13 +192,12 @@ extension JSPromise where Failure: ConstructibleFromJSValue {
         file: StaticString = #file,
         line: Int = #line
     ) -> JSPromise<ResultSuccess, Never> {
-        let closure = JSClosure { arguments -> JSValue in
+        let closure = JSOneshotClosure { arguments -> JSValue in
             guard let error = Failure.construct(from: arguments[0]) else {
                 fatalError("\(file):\(line): failed to unwrap error value for `catch` callback")
             }
             return failure(error).jsValue()
         }
-        callbacks.append(closure)
         return .init(unsafe: jsObject.then!(JSValue.undefined, closure).object!)
     }
 
@@ -222,13 +208,13 @@ extension JSPromise where Failure: ConstructibleFromJSValue {
         file: StaticString = #file,
         line: Int = #line
     ) {
-        let closure = JSClosure { arguments -> () in
+        let closure = JSOneshotClosure { arguments in
             guard let error = Failure.construct(from: arguments[0]) else {
                 fatalError("\(file):\(line): failed to unwrap error value for `catch` callback")
             }
             failure(error)
+            return .undefined
         }
-        callbacks.append(closure)
         _ = jsObject.then!(JSValue.undefined, closure)
     }
 
@@ -241,13 +227,12 @@ extension JSPromise where Failure: ConstructibleFromJSValue {
         file: StaticString = #file,
         line: Int = #line
     ) -> JSPromise<ResultSuccess, ResultFailure> {
-        let closure = JSClosure { arguments -> JSValue in
+        let closure = JSOneshotClosure { arguments -> JSValue in
             guard let error = Failure.construct(from: arguments[0]) else {
                 fatalError("\(file):\(line): failed to unwrap error value for `catch` callback")
             }
             return failure(error).jsValue()
         }
-        callbacks.append(closure)
         return .init(unsafe: jsObject.then!(JSValue.undefined, closure).object!)
     }
 }