From 4c9b87635c36635864b35e046946961f7e3478fc Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Sun, 3 Jan 2021 01:34:07 +0900 Subject: [PATCH 01/11] Major change on JSClosure - Add `JSOneshotClosure` which provide an ability to invoke a closure at most once. - Remove `JSClosure.init(_ body: @escaping ([JSValue]) -> Void)` overload, which forces users to write result type in closure. - Add JSClosure lifetime test suites --- .../Sources/PrimaryTests/main.swift | 39 ++++ .../BasicObjects/JSPromise.swift | 21 +- .../JavaScriptKit/BasicObjects/JSTimer.swift | 5 +- .../FundamentalObjects/JSClosure.swift | 127 ++++++++++++ .../FundamentalObjects/JSFunction.swift | 185 +----------------- .../JSThrowingFunction.swift | 65 ++++++ 6 files changed, 251 insertions(+), 191 deletions(-) create mode 100644 Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift create mode 100644 Sources/JavaScriptKit/FundamentalObjects/JSThrowingFunction.swift diff --git a/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift b/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift index 66d39592e..00c6c1981 100644 --- a/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift +++ b/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift @@ -183,6 +183,45 @@ try test("Function Call") { try expectEqual(func6(true, "OK", 2), .string("OK")) } +try test("Closure Lifetime") { + do { + let c1 = JSClosure { arguments in + return arguments[0] + } + try expectEqual(c1(arguments: [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 c1.throws()) + } + + do { + let c1 = JSClosure { _ in + // JSClosure will be deallocated before `release()` + _ = JSClosure { _ in .undefined } + return .undefined + } + _ = try expectThrow(try c1.throws()) + c1.release() + } + + do { + let c1 = JSOneshotClosure { _ in + return .boolean(true) + } + try expectEqual(c1(), .boolean(true)) + // second call will cause fatalError that can be catched as a JavaScript exception + _ = try expectThrow(try c1.throws()) + // OneshotClosure won't call fatalError even if it's deallocated before `release` + } +} + try test("Host Function Registration") { // ```js // global.globalObject1 = { diff --git a/Sources/JavaScriptKit/BasicObjects/JSPromise.swift b/Sources/JavaScriptKit/BasicObjects/JSPromise.swift index dd46ac47b..c8a9a9113 100644 --- a/Sources/JavaScriptKit/BasicObjects/JSPromise.swift +++ b/Sources/JavaScriptKit/BasicObjects/JSPromise.swift @@ -52,7 +52,10 @@ public final class JSPromise: ConvertibleToJSValue, Constructi /** Schedules the `success` closure to be invoked on sucessful completion of `self`. */ public func then(success: @escaping () -> ()) { - let closure = JSClosure { _ in success() } + let closure = JSClosure { _ in + success() + return .undefined + } callbacks.append(closure) _ = jsObject.then!(closure) } @@ -63,6 +66,7 @@ public final class JSPromise: ConvertibleToJSValue, Constructi public func finally(successOrFailure: @escaping () -> ()) -> Self { let closure = JSClosure { _ in successOrFailure() + return .undefined } callbacks.append(closure) return .init(unsafe: jsObject.finally!(closure).object!) @@ -78,10 +82,11 @@ extension JSPromise where Success == (), Failure == Never { 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 = JSClosure { 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 +98,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) -> ()) -> ()) { - let closure = JSClosure { arguments -> () in + let closure = JSClosure { 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,6 +112,7 @@ extension JSPromise where Failure: ConvertibleToJSValue { reject(error.jsValue()) } } + return .undefined } self.init(unsafe: JSObject.global.Promise.function!.new(closure)) callbacks.append(closure) @@ -118,7 +124,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) -> ()) -> ()) { - let closure = JSClosure { arguments -> () in + let closure = JSClosure { 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,6 +138,7 @@ 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,11 +153,12 @@ extension JSPromise where Success: ConstructibleFromJSValue { file: StaticString = #file, line: Int = #line ) { - let closure = JSClosure { arguments -> () in + let closure = JSClosure { 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) @@ -222,11 +230,12 @@ extension JSPromise where Failure: ConstructibleFromJSValue { file: StaticString = #file, line: Int = #line ) { - let closure = JSClosure { arguments -> () in + let closure = JSClosure { 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) diff --git a/Sources/JavaScriptKit/BasicObjects/JSTimer.swift b/Sources/JavaScriptKit/BasicObjects/JSTimer.swift index aced20b6b..51bec9a7e 100644 --- a/Sources/JavaScriptKit/BasicObjects/JSTimer.swift +++ b/Sources/JavaScriptKit/BasicObjects/JSTimer.swift @@ -34,7 +34,10 @@ public final class JSTimer { - callback: the closure to be executed after a given `millisecondsDelay` interval. */ public init(millisecondsDelay: Double, isRepeating: Bool = false, callback: @escaping () -> ()) { - closure = JSClosure { _ in callback() } + closure = JSClosure { _ in + callback() + return .undefined + } self.isRepeating = isRepeating if isRepeating { value = global.setInterval.function!(closure, millisecondsDelay) diff --git a/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift b/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift new file mode 100644 index 000000000..0cc6671a2 --- /dev/null +++ b/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift @@ -0,0 +1,127 @@ +import _CJavaScriptKit + +fileprivate var sharedFunctions: [JavaScriptHostFuncRef: ([JSValue]) -> JSValue] = [:] + +/// `JSOneshotClosure` is a JavaScript function that can be called at once. +public class JSOneshotClosure: JSFunction { + private var hostFuncRef: JavaScriptHostFuncRef = 0 + + public init(_ body: @escaping ([JSValue]) -> JSValue) { + // 1. Fill `id` as zero at first to access `self` to get `ObjectIdentifier`. + super.init(id: 0) + let objectId = ObjectIdentifier(self) + let funcRef = JavaScriptHostFuncRef(bitPattern: Int32(objectId.hashValue)) + // 2. Retain the given body in static storage by `funcRef`. + sharedFunctions[funcRef] = body + // 3. Create a new JavaScript function which calls the given Swift function. + var objectRef: JavaScriptObjectRef = 0 + _create_function(funcRef, &objectRef) + + hostFuncRef = funcRef + id = objectRef + } + + public override func callAsFunction(this: JSObject? = nil, arguments: [ConvertibleToJSValue]) -> JSValue { + defer { release() } + return super.callAsFunction(this: this, arguments: arguments) + } + + /// Release this function resource. + /// After calling `release`, calling this function from JavaScript will fail. + public func release() { + sharedFunctions[hostFuncRef] = nil + } +} + +/// `JSClosure` represents a JavaScript function the body of which is written in Swift. +/// This type can be passed as a callback handler to JavaScript functions. +/// Note that the lifetime of `JSClosure` should be managed by users manually +/// due to GC boundary between Swift and JavaScript. +/// For further discussion, see also [swiftwasm/JavaScriptKit #33](https://github.com/swiftwasm/JavaScriptKit/pull/33) +/// +/// e.g. +/// ```swift +/// let eventListenter = JSClosure { _ in +/// ... +/// return JSValue.undefined +/// } +/// +/// button.addEventListener!("click", JSValue.function(eventListenter)) +/// ... +/// button.removeEventListener!("click", JSValue.function(eventListenter)) +/// eventListenter.release() +/// ``` +/// +public class JSClosure: JSOneshotClosure { + + var isReleased: Bool = false + + public override func callAsFunction(this: JSObject? = nil, arguments: [ConvertibleToJSValue]) -> JSValue { + try! invokeJSFunction(self, arguments: arguments, this: this) + } + + public override func release() { + isReleased = true + super.release() + } + + deinit { + guard isReleased else { + fatalError(""" + release() must be called on closures manually before deallocating. + This is caused by the lack of support for the `FinalizationRegistry` API in Safari. + """) + } + } +} + +// MARK: - `JSClosure` mechanism note +// +// 1. Create thunk function in JavaScript world, that has a reference +// to Swift Closure. +// ┌─────────────────────┬──────────────────────────┐ +// │ Swift side │ JavaScript side │ +// │ │ │ +// │ │ │ +// │ │ ┌──[Thunk function]──┐ │ +// │ ┌ ─ ─ ─ ─ ─│─ ─│─ ─ ─ ─ ─ ┐ │ │ +// │ ↓ │ │ │ │ │ +// │ [Swift Closure] │ │ Host Function ID │ │ +// │ │ │ │ │ +// │ │ └────────────────────┘ │ +// └─────────────────────┴──────────────────────────┘ +// +// 2. When thunk function is invoked, it calls Swift Closure via +// `_call_host_function` and callback the result through callback func +// ┌─────────────────────┬──────────────────────────┐ +// │ Swift side │ JavaScript side │ +// │ │ │ +// │ │ │ +// │ Apply ┌──[Thunk function]──┐ │ +// │ ┌ ─ ─ ─ ─ ─│─ ─│─ ─ ─ ─ ─ ┐ │ │ +// │ ↓ │ │ │ │ │ +// │ [Swift Closure] │ │ Host Function ID │ │ +// │ │ │ │ │ │ +// │ │ │ └────────────────────┘ │ +// │ │ │ ↑ │ +// │ │ Apply │ │ +// │ └─[Result]─┼───>[Callback func]─┘ │ +// │ │ │ +// └─────────────────────┴──────────────────────────┘ + +@_cdecl("_call_host_function_impl") +func _call_host_function_impl( + _ hostFuncRef: JavaScriptHostFuncRef, + _ argv: UnsafePointer, _ argc: Int32, + _ callbackFuncRef: JavaScriptObjectRef +) { + guard let hostFunc = sharedFunctions[hostFuncRef] else { + fatalError("The function was already released") + } + let arguments = UnsafeBufferPointer(start: argv, count: Int(argc)).map { + $0.jsValue() + } + let result = hostFunc(arguments) + let callbackFuncRef = JSFunction(id: callbackFuncRef) + _ = callbackFuncRef(result) +} diff --git a/Sources/JavaScriptKit/FundamentalObjects/JSFunction.swift b/Sources/JavaScriptKit/FundamentalObjects/JSFunction.swift index 89bf168e3..d5e819f94 100644 --- a/Sources/JavaScriptKit/FundamentalObjects/JSFunction.swift +++ b/Sources/JavaScriptKit/FundamentalObjects/JSFunction.swift @@ -88,70 +88,7 @@ public class JSFunction: JSObject { } } -/// A `JSFunction` wrapper that enables throwing function calls. -/// Exceptions produced by JavaScript functions will be thrown as `JSValue`. -public class JSThrowingFunction { - private let base: JSFunction - public init(_ base: JSFunction) { - self.base = base - } - - /// Call this function with given `arguments` and binding given `this` as context. - /// - Parameters: - /// - this: The value to be passed as the `this` parameter to this function. - /// - arguments: Arguments to be passed to this function. - /// - Returns: The result of this call. - @discardableResult - public func callAsFunction(this: JSObject? = nil, arguments: [ConvertibleToJSValue]) throws -> JSValue { - try invokeJSFunction(base, arguments: arguments, this: this) - } - - /// A variadic arguments version of `callAsFunction`. - @discardableResult - public func callAsFunction(this: JSObject? = nil, _ arguments: ConvertibleToJSValue...) throws -> JSValue { - try self(this: this, arguments: arguments) - } - - /// Instantiate an object from this function as a throwing constructor. - /// - /// Guaranteed to return an object because either: - /// - /// - a. the constructor explicitly returns an object, or - /// - b. the constructor returns nothing, which causes JS to return the `this` value, or - /// - c. the constructor returns undefined, null or a non-object, in which case JS also returns `this`. - /// - /// - Parameter arguments: Arguments to be passed to this constructor function. - /// - Returns: A new instance of this constructor. - public func new(arguments: [ConvertibleToJSValue]) throws -> JSObject { - try arguments.withRawJSValues { rawValues -> Result in - rawValues.withUnsafeBufferPointer { bufferPointer in - let argv = bufferPointer.baseAddress - let argc = bufferPointer.count - - var exceptionKind = JavaScriptValueKindAndFlags() - var exceptionPayload1 = JavaScriptPayload1() - var exceptionPayload2 = JavaScriptPayload2() - var resultObj = JavaScriptObjectRef() - _call_throwing_new( - self.base.id, argv, Int32(argc), - &resultObj, &exceptionKind, &exceptionPayload1, &exceptionPayload2 - ) - if exceptionKind.isException { - let exception = RawJSValue(kind: exceptionKind.kind, payload1: exceptionPayload1, payload2: exceptionPayload2) - return .failure(exception.jsValue()) - } - return .success(JSObject(id: resultObj)) - } - }.get() - } - - /// A variadic arguments version of `new`. - public func new(_ arguments: ConvertibleToJSValue...) throws -> JSObject { - try new(arguments: arguments) - } -} - -fileprivate func invokeJSFunction(_ jsFunc: JSFunction, arguments: [ConvertibleToJSValue], this: JSObject?) throws -> JSValue { +internal func invokeJSFunction(_ jsFunc: JSFunction, arguments: [ConvertibleToJSValue], this: JSObject?) throws -> JSValue { let (result, isException) = arguments.withRawJSValues { rawValues in rawValues.withUnsafeBufferPointer { bufferPointer -> (JSValue, Bool) in let argv = bufferPointer.baseAddress @@ -178,123 +115,3 @@ fileprivate func invokeJSFunction(_ jsFunc: JSFunction, arguments: [ConvertibleT } return result } - -/// `JSClosure` represents a JavaScript function the body of which is written in Swift. -/// This type can be passed as a callback handler to JavaScript functions. -/// Note that the lifetime of `JSClosure` should be managed by users manually -/// due to GC boundary between Swift and JavaScript. -/// For further discussion, see also [swiftwasm/JavaScriptKit #33](https://github.com/swiftwasm/JavaScriptKit/pull/33) -/// -/// e.g. -/// ```swift -/// let eventListenter = JSClosure { _ in -/// ... -/// return JSValue.undefined -/// } -/// -/// button.addEventListener!("click", JSValue.function(eventListenter)) -/// ... -/// button.removeEventListener!("click", JSValue.function(eventListenter)) -/// eventListenter.release() -/// ``` -/// -public class JSClosure: JSFunction { - static var sharedFunctions: [JavaScriptHostFuncRef: ([JSValue]) -> JSValue] = [:] - - private var hostFuncRef: JavaScriptHostFuncRef = 0 - - private var isReleased = false - - /// Instantiate a new `JSClosure` with given function body. - /// - Parameter body: The body of this function. - public init(_ body: @escaping ([JSValue]) -> JSValue) { - // 1. Fill `id` as zero at first to access `self` to get `ObjectIdentifier`. - super.init(id: 0) - let objectId = ObjectIdentifier(self) - let funcRef = JavaScriptHostFuncRef(bitPattern: Int32(objectId.hashValue)) - // 2. Retain the given body in static storage by `funcRef`. - Self.sharedFunctions[funcRef] = body - // 3. Create a new JavaScript function which calls the given Swift function. - var objectRef: JavaScriptObjectRef = 0 - _create_function(funcRef, &objectRef) - - hostFuncRef = funcRef - id = objectRef - } - - /// A convenience initializer which assumes that the given body function returns `JSValue.undefined` - convenience public init(_ body: @escaping ([JSValue]) -> ()) { - self.init { (arguments: [JSValue]) -> JSValue in - body(arguments) - return .undefined - } - } - - /// Release this function resource. - /// After calling `release`, calling this function from JavaScript will fail. - public func release() { - Self.sharedFunctions[hostFuncRef] = nil - isReleased = true - } - - deinit { - guard isReleased else { - fatalError(""" - release() must be called on closures manually before deallocating. - This is caused by the lack of support for the `FinalizationRegistry` API in Safari. - """) - } - } -} - - -// MARK: - `JSClosure` mechanism note -// -// 1. Create thunk function in JavaScript world, that has a reference -// to Swift Closure. -// ┌─────────────────────┬──────────────────────────┐ -// │ Swift side │ JavaScript side │ -// │ │ │ -// │ │ │ -// │ │ ┌──[Thunk function]──┐ │ -// │ ┌ ─ ─ ─ ─ ─│─ ─│─ ─ ─ ─ ─ ┐ │ │ -// │ ↓ │ │ │ │ │ -// │ [Swift Closure] │ │ Host Function ID │ │ -// │ │ │ │ │ -// │ │ └────────────────────┘ │ -// └─────────────────────┴──────────────────────────┘ -// -// 2. When thunk function is invoked, it calls Swift Closure via -// `_call_host_function` and callback the result through callback func -// ┌─────────────────────┬──────────────────────────┐ -// │ Swift side │ JavaScript side │ -// │ │ │ -// │ │ │ -// │ Apply ┌──[Thunk function]──┐ │ -// │ ┌ ─ ─ ─ ─ ─│─ ─│─ ─ ─ ─ ─ ┐ │ │ -// │ ↓ │ │ │ │ │ -// │ [Swift Closure] │ │ Host Function ID │ │ -// │ │ │ │ │ │ -// │ │ │ └────────────────────┘ │ -// │ │ │ ↑ │ -// │ │ Apply │ │ -// │ └─[Result]─┼───>[Callback func]─┘ │ -// │ │ │ -// └─────────────────────┴──────────────────────────┘ - -@_cdecl("_call_host_function_impl") -func _call_host_function_impl( - _ hostFuncRef: JavaScriptHostFuncRef, - _ argv: UnsafePointer, _ argc: Int32, - _ callbackFuncRef: JavaScriptObjectRef -) { - guard let hostFunc = JSClosure.sharedFunctions[hostFuncRef] else { - fatalError("The function was already released") - } - let arguments = UnsafeBufferPointer(start: argv, count: Int(argc)).map { - $0.jsValue() - } - let result = hostFunc(arguments) - let callbackFuncRef = JSFunction(id: callbackFuncRef) - _ = callbackFuncRef(result) -} diff --git a/Sources/JavaScriptKit/FundamentalObjects/JSThrowingFunction.swift b/Sources/JavaScriptKit/FundamentalObjects/JSThrowingFunction.swift new file mode 100644 index 000000000..ffd6f6894 --- /dev/null +++ b/Sources/JavaScriptKit/FundamentalObjects/JSThrowingFunction.swift @@ -0,0 +1,65 @@ +import _CJavaScriptKit + + +/// A `JSFunction` wrapper that enables throwing function calls. +/// Exceptions produced by JavaScript functions will be thrown as `JSValue`. +public class JSThrowingFunction { + private let base: JSFunction + public init(_ base: JSFunction) { + self.base = base + } + + /// Call this function with given `arguments` and binding given `this` as context. + /// - Parameters: + /// - this: The value to be passed as the `this` parameter to this function. + /// - arguments: Arguments to be passed to this function. + /// - Returns: The result of this call. + @discardableResult + public func callAsFunction(this: JSObject? = nil, arguments: [ConvertibleToJSValue]) throws -> JSValue { + try invokeJSFunction(base, arguments: arguments, this: this) + } + + /// A variadic arguments version of `callAsFunction`. + @discardableResult + public func callAsFunction(this: JSObject? = nil, _ arguments: ConvertibleToJSValue...) throws -> JSValue { + try self(this: this, arguments: arguments) + } + + /// Instantiate an object from this function as a throwing constructor. + /// + /// Guaranteed to return an object because either: + /// + /// - a. the constructor explicitly returns an object, or + /// - b. the constructor returns nothing, which causes JS to return the `this` value, or + /// - c. the constructor returns undefined, null or a non-object, in which case JS also returns `this`. + /// + /// - Parameter arguments: Arguments to be passed to this constructor function. + /// - Returns: A new instance of this constructor. + public func new(arguments: [ConvertibleToJSValue]) throws -> JSObject { + try arguments.withRawJSValues { rawValues -> Result in + rawValues.withUnsafeBufferPointer { bufferPointer in + let argv = bufferPointer.baseAddress + let argc = bufferPointer.count + + var exceptionKind = JavaScriptValueKindAndFlags() + var exceptionPayload1 = JavaScriptPayload1() + var exceptionPayload2 = JavaScriptPayload2() + var resultObj = JavaScriptObjectRef() + _call_throwing_new( + self.base.id, argv, Int32(argc), + &resultObj, &exceptionKind, &exceptionPayload1, &exceptionPayload2 + ) + if exceptionKind.isException { + let exception = RawJSValue(kind: exceptionKind.kind, payload1: exceptionPayload1, payload2: exceptionPayload2) + return .failure(exception.jsValue()) + } + return .success(JSObject(id: resultObj)) + } + }.get() + } + + /// A variadic arguments version of `new`. + public func new(_ arguments: ConvertibleToJSValue...) throws -> JSObject { + try new(arguments: arguments) + } +} From df7ff629a5314a199bfd35801789259c02a31d02 Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Sun, 3 Jan 2021 02:02:20 +0900 Subject: [PATCH 02/11] Keep JSClosure overloads as derepcated initializer --- .../FundamentalObjects/JSClosure.swift | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift b/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift index 0cc6671a2..fa9a83e72 100644 --- a/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift +++ b/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift @@ -56,6 +56,19 @@ public class JSClosure: JSOneshotClosure { var isReleased: Bool = false + @available(*, deprecated, message: "This initializer will be removed in the next minor version update. Please use `init(_ body: @escaping ([JSValue]) -> JSValue)`") + @_disfavoredOverload + public init(_ body: @escaping ([JSValue]) -> ()) { + super.init({ + body($0) + return .undefined + }) + } + + public override init(_ body: @escaping ([JSValue]) -> JSValue) { + super.init(body) + } + public override func callAsFunction(this: JSObject? = nil, arguments: [ConvertibleToJSValue]) -> JSValue { try! invokeJSFunction(self, arguments: arguments, this: this) } From e67b03ee1efa48a243f1d43181bd9d8becd9fa3a Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Sun, 3 Jan 2021 02:04:50 +0900 Subject: [PATCH 03/11] Apply suggestions from code review Co-authored-by: Max Desiatov --- IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift | 2 +- Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift b/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift index 00c6c1981..e4d990a55 100644 --- a/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift +++ b/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift @@ -216,7 +216,7 @@ try test("Closure Lifetime") { return .boolean(true) } try expectEqual(c1(), .boolean(true)) - // second call will cause fatalError that can be catched as a JavaScript exception + // second call will cause `fatalError` that can be caught as a JavaScript exception _ = try expectThrow(try c1.throws()) // OneshotClosure won't call fatalError even if it's deallocated before `release` } diff --git a/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift b/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift index fa9a83e72..e10234222 100644 --- a/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift +++ b/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift @@ -2,7 +2,7 @@ import _CJavaScriptKit fileprivate var sharedFunctions: [JavaScriptHostFuncRef: ([JSValue]) -> JSValue] = [:] -/// `JSOneshotClosure` is a JavaScript function that can be called at once. +/// `JSOneshotClosure` is a JavaScript function that can be called only once. public class JSOneshotClosure: JSFunction { private var hostFuncRef: JavaScriptHostFuncRef = 0 From 65658bdd6cd4577d0c293c8538d78500bc9c6278 Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Sun, 3 Jan 2021 22:11:59 +0900 Subject: [PATCH 04/11] Update Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift Co-authored-by: Jed Fox --- .../JavaScriptKit/FundamentalObjects/JSClosure.swift | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift b/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift index e10234222..1fdcfcab9 100644 --- a/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift +++ b/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift @@ -90,13 +90,13 @@ public class JSClosure: JSOneshotClosure { // MARK: - `JSClosure` mechanism note // -// 1. Create thunk function in JavaScript world, that has a reference -// to Swift Closure. +// 1. Create a thunk in the JavaScript world, which has a reference +// to a Swift closure. // ┌─────────────────────┬──────────────────────────┐ // │ Swift side │ JavaScript side │ // │ │ │ // │ │ │ -// │ │ ┌──[Thunk function]──┐ │ +// │ │ ┌──────[Thunk]───────┐ │ // │ ┌ ─ ─ ─ ─ ─│─ ─│─ ─ ─ ─ ─ ┐ │ │ // │ ↓ │ │ │ │ │ // │ [Swift Closure] │ │ Host Function ID │ │ @@ -104,13 +104,13 @@ public class JSClosure: JSOneshotClosure { // │ │ └────────────────────┘ │ // └─────────────────────┴──────────────────────────┘ // -// 2. When thunk function is invoked, it calls Swift Closure via -// `_call_host_function` and callback the result through callback func +// 2. When the thunk function is invoked, it calls the Swift closure via +// `_call_host_function` and receives the result through a callback. // ┌─────────────────────┬──────────────────────────┐ // │ Swift side │ JavaScript side │ // │ │ │ // │ │ │ -// │ Apply ┌──[Thunk function]──┐ │ +// │ Apply ┌──────[Thunk]───────┐ │ // │ ┌ ─ ─ ─ ─ ─│─ ─│─ ─ ─ ─ ─ ┐ │ │ // │ ↓ │ │ │ │ │ // │ [Swift Closure] │ │ Host Function ID │ │ From 3b9392487731439cc48b86c5a1074271708d47af Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Mon, 4 Jan 2021 01:17:09 +0900 Subject: [PATCH 05/11] Apply suggestions from code review Co-authored-by: Jed Fox --- Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift b/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift index 1fdcfcab9..2ab31bc99 100644 --- a/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift +++ b/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift @@ -56,7 +56,7 @@ public class JSClosure: JSOneshotClosure { var isReleased: Bool = false - @available(*, deprecated, message: "This initializer will be removed in the next minor version update. Please use `init(_ body: @escaping ([JSValue]) -> JSValue)`") + @available(*, deprecated, message: "This initializer will be removed in the next minor version update. Please use `init(_ body: @escaping ([JSValue]) -> JSValue)` and add `return .undefined` to the end of your closure") @_disfavoredOverload public init(_ body: @escaping ([JSValue]) -> ()) { super.init({ @@ -80,10 +80,8 @@ public class JSClosure: JSOneshotClosure { deinit { guard isReleased else { - fatalError(""" - release() must be called on closures manually before deallocating. - This is caused by the lack of support for the `FinalizationRegistry` API in Safari. - """) + // Safari doesn't support `FinalizationRegistry`, so we cannot automatically manage the lifetime of Swift objects + fatalError("release() must be called on JSClosure objects manually before they are deallocated") } } } From efd633e427c8d29086d0c044246bfdcbf8f1dfef Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Mon, 4 Jan 2021 01:39:01 +0900 Subject: [PATCH 06/11] Use JSOneshotClosure in JSTimer --- .../TestSuites/Sources/PrimaryTests/main.swift | 12 ++++++++++-- Sources/JavaScriptKit/BasicObjects/JSTimer.swift | 15 +++++++++++---- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift b/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift index e4d990a55..27d710364 100644 --- a/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift +++ b/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift @@ -488,21 +488,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) diff --git a/Sources/JavaScriptKit/BasicObjects/JSTimer.swift b/Sources/JavaScriptKit/BasicObjects/JSTimer.swift index 51bec9a7e..a8f3c96a4 100644 --- a/Sources/JavaScriptKit/BasicObjects/JSTimer.swift +++ b/Sources/JavaScriptKit/BasicObjects/JSTimer.swift @@ -14,7 +14,7 @@ public final class JSTimer { /// Indicates whether this timer instance calls its callback repeatedly at a given delay. public let isRepeating: Bool - private let closure: JSClosure + private let closure: JSOneshotClosure /** Node.js and browser APIs are slightly different. `setTimeout`/`setInterval` return an object in Node.js, while browsers return a number. Fortunately, clearTimeout and clearInterval take @@ -34,9 +34,16 @@ public final class JSTimer { - callback: the closure to be executed after a given `millisecondsDelay` interval. */ public init(millisecondsDelay: Double, isRepeating: Bool = false, callback: @escaping () -> ()) { - closure = JSClosure { _ in - callback() - return .undefined + if isRepeating { + closure = JSClosure { _ in + callback() + return .undefined + } + } else { + closure = JSOneshotClosure { _ in + callback() + return .undefined + } } self.isRepeating = isRepeating if isRepeating { From 76193c7ee7993774d384900d25578a3a3662d038 Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Mon, 4 Jan 2021 02:07:21 +0900 Subject: [PATCH 07/11] Remove JSClosure.callAsFunction --- .../Sources/PrimaryTests/main.swift | 20 ++-- IntegrationTests/bin/primary-tests.js | 3 + .../JavaScriptKit/BasicObjects/JSTimer.swift | 96 +++++++++---------- .../FundamentalObjects/JSClosure.swift | 57 ++++++----- Sources/JavaScriptKit/JSValue.swift | 2 +- 5 files changed, 98 insertions(+), 80 deletions(-) diff --git a/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift b/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift index 27d710364..66daeec0a 100644 --- a/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift +++ b/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift @@ -183,12 +183,14 @@ 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(c1(arguments: [JSValue.number(1.0)]), .number(1.0)) + try expectEqual(evalClosure(c1, JSValue.number(1.0)), .number(1.0)) c1.release() } @@ -198,7 +200,7 @@ try test("Closure Lifetime") { } c1.release() // Call a released closure - _ = try expectThrow(try c1.throws()) + _ = try expectThrow(try evalClosure.throws(c1)) } do { @@ -207,7 +209,7 @@ try test("Closure Lifetime") { _ = JSClosure { _ in .undefined } return .undefined } - _ = try expectThrow(try c1.throws()) + _ = try expectThrow(try evalClosure.throws(c1)) c1.release() } @@ -215,9 +217,9 @@ try test("Closure Lifetime") { let c1 = JSOneshotClosure { _ in return .boolean(true) } - try expectEqual(c1(), .boolean(true)) + try expectEqual(evalClosure(c1), .boolean(true)) // second call will cause `fatalError` that can be caught as a JavaScript exception - _ = try expectThrow(try c1.throws()) + _ = try expectThrow(try evalClosure.throws(c1)) // OneshotClosure won't call fatalError even if it's deallocated before `release` } } @@ -244,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) @@ -262,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() } @@ -375,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() diff --git a/IntegrationTests/bin/primary-tests.js b/IntegrationTests/bin/primary-tests.js index b1f1049d1..6cb3aea3c 100644 --- a/IntegrationTests/bin/primary-tests.js +++ b/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) { diff --git a/Sources/JavaScriptKit/BasicObjects/JSTimer.swift b/Sources/JavaScriptKit/BasicObjects/JSTimer.swift index a8f3c96a4..228b7e83d 100644 --- a/Sources/JavaScriptKit/BasicObjects/JSTimer.swift +++ b/Sources/JavaScriptKit/BasicObjects/JSTimer.swift @@ -1,5 +1,5 @@ /** This timer is an abstraction over [`setInterval`](https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setInterval) -/ [`clearInterval`](https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/clearInterval) and +/ [`clearInterval`](https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/clearInterval) and [`setTimeout`](https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setTimeout) / [`clearTimeout`](https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setTimeout) JavaScript functions. It intentionally doesn't match the JavaScript API, as a special care is @@ -7,62 +7,62 @@ needed to hold a reference to the timer closure and to call `JSClosure.release() timer is deallocated. As a user, you have to hold a reference to a `JSTimer` instance for it to stay valid. The `JSTimer` API is also intentionally trivial, the timer is started right away, and the only way to invalidate the timer is to bring the reference count of the `JSTimer` instance to zero. -For invalidation you should either store the timer in an optional property and assign `nil` to it, +For invalidation you should either store the timer in an optional property and assign `nil` to it, or deallocate the object that owns the timer. */ public final class JSTimer { - /// Indicates whether this timer instance calls its callback repeatedly at a given delay. - public let isRepeating: Bool + /// Indicates whether this timer instance calls its callback repeatedly at a given delay. + public let isRepeating: Bool - private let closure: JSOneshotClosure + private let closure: JSClosureProtocol - /** Node.js and browser APIs are slightly different. `setTimeout`/`setInterval` return an object - in Node.js, while browsers return a number. Fortunately, clearTimeout and clearInterval take - corresponding types as their arguments, and we can store either as JSValue, so we can treat both - cases uniformly. - */ - private let value: JSValue - private let global = JSObject.global + /** Node.js and browser APIs are slightly different. `setTimeout`/`setInterval` return an object + in Node.js, while browsers return a number. Fortunately, clearTimeout and clearInterval take + corresponding types as their arguments, and we can store either as JSValue, so we can treat both + cases uniformly. + */ + private let value: JSValue + private let global = JSObject.global - /** - Creates a new timer instance that calls `setInterval` or `setTimeout` JavaScript functions for you - under the hood. - - Parameters: - - millisecondsDelay: the amount of milliseconds before the `callback` closure is executed. - - isRepeating: when `true` the `callback` closure is executed repeatedly at given - `millisecondsDelay` intervals indefinitely until the timer is deallocated. - - callback: the closure to be executed after a given `millisecondsDelay` interval. - */ - public init(millisecondsDelay: Double, isRepeating: Bool = false, callback: @escaping () -> ()) { - if isRepeating { - closure = JSClosure { _ in - callback() - return .undefined + /** + Creates a new timer instance that calls `setInterval` or `setTimeout` JavaScript functions for you + under the hood. + - Parameters: + - millisecondsDelay: the amount of milliseconds before the `callback` closure is executed. + - isRepeating: when `true` the `callback` closure is executed repeatedly at given + `millisecondsDelay` intervals indefinitely until the timer is deallocated. + - callback: the closure to be executed after a given `millisecondsDelay` interval. + */ + public init(millisecondsDelay: Double, isRepeating: Bool = false, callback: @escaping () -> ()) { + if isRepeating { + closure = JSClosure { _ in + callback() + return .undefined + } + } else { + closure = JSOneshotClosure { _ in + callback() + return .undefined + } } - } else { - closure = JSOneshotClosure { _ in - callback() - return .undefined + self.isRepeating = isRepeating + if isRepeating { + value = global.setInterval.function!(closure, millisecondsDelay) + } else { + value = global.setTimeout.function!(closure, millisecondsDelay) } } - self.isRepeating = isRepeating - if isRepeating { - value = global.setInterval.function!(closure, millisecondsDelay) - } else { - value = global.setTimeout.function!(closure, millisecondsDelay) - } - } - /** Makes a corresponding `clearTimeout` or `clearInterval` call, depending on whether this timer - instance is repeating. The `closure` instance is released manually here, as it is required for - bridged closure instances. - */ - deinit { - if isRepeating { - global.clearInterval.function!(value) - } else { - global.clearTimeout.function!(value) + /** Makes a corresponding `clearTimeout` or `clearInterval` call, depending on whether this timer + instance is repeating. The `closure` instance is released manually here, as it is required for + bridged closure instances. + */ + deinit { + if isRepeating { + global.clearInterval.function!(value) + } else { + global.clearTimeout.function!(value) + } + closure.release() } - closure.release() - } } diff --git a/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift b/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift index 2ab31bc99..cd96dd0b5 100644 --- a/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift +++ b/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift @@ -1,9 +1,17 @@ import _CJavaScriptKit -fileprivate var sharedFunctions: [JavaScriptHostFuncRef: ([JSValue]) -> JSValue] = [:] +fileprivate var sharedClosures: [JavaScriptHostFuncRef: ([JSValue]) -> JSValue] = [:] + +/// JSClosureProtocol abstracts closure object in JavaScript, whose lifetime is manualy managed +public protocol JSClosureProtocol: JSValueCompatible { + + /// Release this function resource. + /// After calling `release`, calling this function from JavaScript will fail. + func release() +} /// `JSOneshotClosure` is a JavaScript function that can be called only once. -public class JSOneshotClosure: JSFunction { +public class JSOneshotClosure: JSObject, JSClosureProtocol { private var hostFuncRef: JavaScriptHostFuncRef = 0 public init(_ body: @escaping ([JSValue]) -> JSValue) { @@ -12,7 +20,10 @@ public class JSOneshotClosure: JSFunction { let objectId = ObjectIdentifier(self) let funcRef = JavaScriptHostFuncRef(bitPattern: Int32(objectId.hashValue)) // 2. Retain the given body in static storage by `funcRef`. - sharedFunctions[funcRef] = body + sharedClosures[funcRef] = { + defer { self.release() } + return body($0) + } // 3. Create a new JavaScript function which calls the given Swift function. var objectRef: JavaScriptObjectRef = 0 _create_function(funcRef, &objectRef) @@ -21,15 +32,10 @@ public class JSOneshotClosure: JSFunction { id = objectRef } - public override func callAsFunction(this: JSObject? = nil, arguments: [ConvertibleToJSValue]) -> JSValue { - defer { release() } - return super.callAsFunction(this: this, arguments: arguments) - } - /// Release this function resource. /// After calling `release`, calling this function from JavaScript will fail. public func release() { - sharedFunctions[hostFuncRef] = nil + sharedClosures[hostFuncRef] = nil } } @@ -52,30 +58,37 @@ public class JSOneshotClosure: JSFunction { /// eventListenter.release() /// ``` /// -public class JSClosure: JSOneshotClosure { - +public class JSClosure: JSObject, JSClosureProtocol { + private var hostFuncRef: JavaScriptHostFuncRef = 0 var isReleased: Bool = false @available(*, deprecated, message: "This initializer will be removed in the next minor version update. Please use `init(_ body: @escaping ([JSValue]) -> JSValue)` and add `return .undefined` to the end of your closure") @_disfavoredOverload - public init(_ body: @escaping ([JSValue]) -> ()) { - super.init({ + public convenience init(_ body: @escaping ([JSValue]) -> ()) { + self.init({ body($0) return .undefined }) } - public override init(_ body: @escaping ([JSValue]) -> JSValue) { - super.init(body) - } + public init(_ body: @escaping ([JSValue]) -> JSValue) { + // 1. Fill `id` as zero at first to access `self` to get `ObjectIdentifier`. + super.init(id: 0) + let objectId = ObjectIdentifier(self) + let funcRef = JavaScriptHostFuncRef(bitPattern: Int32(objectId.hashValue)) + // 2. Retain the given body in static storage by `funcRef`. + sharedClosures[funcRef] = body + // 3. Create a new JavaScript function which calls the given Swift function. + var objectRef: JavaScriptObjectRef = 0 + _create_function(funcRef, &objectRef) - public override func callAsFunction(this: JSObject? = nil, arguments: [ConvertibleToJSValue]) -> JSValue { - try! invokeJSFunction(self, arguments: arguments, this: this) + hostFuncRef = funcRef + id = objectRef } - - public override func release() { + + public func release() { isReleased = true - super.release() + sharedClosures[hostFuncRef] = nil } deinit { @@ -126,7 +139,7 @@ func _call_host_function_impl( _ argv: UnsafePointer, _ argc: Int32, _ callbackFuncRef: JavaScriptObjectRef ) { - guard let hostFunc = sharedFunctions[hostFuncRef] else { + guard let hostFunc = sharedClosures[hostFuncRef] else { fatalError("The function was already released") } let arguments = UnsafeBufferPointer(start: argv, count: Int(argc)).map { diff --git a/Sources/JavaScriptKit/JSValue.swift b/Sources/JavaScriptKit/JSValue.swift index 438104441..eb14eda82 100644 --- a/Sources/JavaScriptKit/JSValue.swift +++ b/Sources/JavaScriptKit/JSValue.swift @@ -142,7 +142,7 @@ extension JSValue { /// ``` @available(*, deprecated, message: "Please create JSClosure directly and manage its lifetime manually.") public static func function(_ body: @escaping ([JSValue]) -> JSValue) -> JSValue { - .function(JSClosure(body)) + .object(JSClosure(body)) } } From 2f843811161465688d3f3f287c2ec8c7f44bbfb6 Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Mon, 4 Jan 2021 02:11:18 +0900 Subject: [PATCH 08/11] Use JSOneshotClosure instead of JSClosure in JSPromise --- .../BasicObjects/JSPromise.swift | 46 +++++-------------- 1 file changed, 11 insertions(+), 35 deletions(-) diff --git a/Sources/JavaScriptKit/BasicObjects/JSPromise.swift b/Sources/JavaScriptKit/BasicObjects/JSPromise.swift index c8a9a9113..02a6b27c0 100644 --- a/Sources/JavaScriptKit/BasicObjects/JSPromise.swift +++ b/Sources/JavaScriptKit/BasicObjects/JSPromise.swift @@ -8,20 +8,11 @@ is of actual JavaScript `Error` type, you should use `JSPromise: 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,11 +43,10 @@ public final class JSPromise: ConvertibleToJSValue, Constructi /** Schedules the `success` closure to be invoked on sucessful completion of `self`. */ public func then(success: @escaping () -> ()) { - let closure = JSClosure { _ in + let closure = JSOneshotClosure { _ in success() return .undefined } - callbacks.append(closure) _ = jsObject.then!(closure) } @@ -64,17 +54,12 @@ public final class JSPromise: ConvertibleToJSValue, Constructi `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 { @@ -82,14 +67,13 @@ extension JSPromise where Success == (), Failure == Never { 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) } } @@ -98,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) -> ()) -> ()) { - 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! @@ -115,7 +99,6 @@ extension JSPromise where Failure: ConvertibleToJSValue { return .undefined } self.init(unsafe: JSObject.global.Promise.function!.new(closure)) - callbacks.append(closure) } } @@ -124,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) -> ()) -> ()) { - 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! @@ -141,7 +124,6 @@ extension JSPromise where Success: ConvertibleToJSValue, Failure: JSError { return .undefined } self.init(unsafe: JSObject.global.Promise.function!.new(closure)) - callbacks.append(closure) } } @@ -153,14 +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) } @@ -173,13 +154,12 @@ extension JSPromise where Success: ConstructibleFromJSValue { file: StaticString = #file, line: Int = #line ) -> JSPromise { - 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!) } @@ -192,13 +172,12 @@ extension JSPromise where Success: ConstructibleFromJSValue { file: StaticString = #file, line: Int = #line ) -> JSPromise { - 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!) } } @@ -213,13 +192,12 @@ extension JSPromise where Failure: ConstructibleFromJSValue { file: StaticString = #file, line: Int = #line ) -> JSPromise { - 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!) } @@ -230,14 +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) } @@ -250,13 +227,12 @@ extension JSPromise where Failure: ConstructibleFromJSValue { file: StaticString = #file, line: Int = #line ) -> JSPromise { - 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!) } } From 110b72da79ec171f3af0071ace8a6fada562b7be Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Mon, 4 Jan 2021 02:13:34 +0900 Subject: [PATCH 09/11] Fix compatibility build target dir --- .github/workflows/compatibility.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/compatibility.yml b/.github/workflows/compatibility.yml index 40e63d759..e4534507e 100644 --- a/.github/workflows/compatibility.yml +++ b/.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 From aed4312ddf77d1cfe81c186ff352c03cd5f8b505 Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Mon, 4 Jan 2021 02:17:17 +0900 Subject: [PATCH 10/11] Add compatibility method for JSValue.function(_:JSClosure) --- .../Sources/JavaScriptKitExample/main.swift | 2 +- Sources/JavaScriptKit/JSValue.swift | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Example/JavaScriptKitExample/Sources/JavaScriptKitExample/main.swift b/Example/JavaScriptKitExample/Sources/JavaScriptKitExample/main.swift index 58f582a7e..cbd11acf9 100644 --- a/Example/JavaScriptKitExample/Sources/JavaScriptKitExample/main.swift +++ b/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) diff --git a/Sources/JavaScriptKit/JSValue.swift b/Sources/JavaScriptKit/JSValue.swift index eb14eda82..34a210364 100644 --- a/Sources/JavaScriptKit/JSValue.swift +++ b/Sources/JavaScriptKit/JSValue.swift @@ -144,6 +144,11 @@ extension JSValue { public static func function(_ body: @escaping ([JSValue]) -> JSValue) -> JSValue { .object(JSClosure(body)) } + + @available(*, deprecated, renamed: "object", message: "JSClosure is not a subclass of JSFunction now") + public static func function(_ closure: JSClosure) -> JSValue { + .object(closure) + } } extension JSValue: ExpressibleByStringLiteral { From 0896483adc1be794772429fb84ff560ee63b9455 Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Mon, 4 Jan 2021 02:43:48 +0900 Subject: [PATCH 11/11] Update Sources/JavaScriptKit/JSValue.swift Co-authored-by: Jed Fox --- Sources/JavaScriptKit/JSValue.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/JavaScriptKit/JSValue.swift b/Sources/JavaScriptKit/JSValue.swift index 34a210364..34bb78232 100644 --- a/Sources/JavaScriptKit/JSValue.swift +++ b/Sources/JavaScriptKit/JSValue.swift @@ -145,7 +145,7 @@ extension JSValue { .object(JSClosure(body)) } - @available(*, deprecated, renamed: "object", message: "JSClosure is not a subclass of JSFunction now") + @available(*, deprecated, renamed: "object", message: "JSClosure is no longer a subclass of JSFunction. Use .object(closure) instead.") public static func function(_ closure: JSClosure) -> JSValue { .object(closure) }