Skip to content

Commit 76193c7

Browse files
Remove JSClosure.callAsFunction
1 parent efd633e commit 76193c7

File tree

5 files changed

+98
-80
lines changed

5 files changed

+98
-80
lines changed

IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -183,12 +183,14 @@ try test("Function Call") {
183183
try expectEqual(func6(true, "OK", 2), .string("OK"))
184184
}
185185

186+
let evalClosure = JSObject.global.globalObject1.eval_closure.function!
187+
186188
try test("Closure Lifetime") {
187189
do {
188190
let c1 = JSClosure { arguments in
189191
return arguments[0]
190192
}
191-
try expectEqual(c1(arguments: [JSValue.number(1.0)]), .number(1.0))
193+
try expectEqual(evalClosure(c1, JSValue.number(1.0)), .number(1.0))
192194
c1.release()
193195
}
194196

@@ -198,7 +200,7 @@ try test("Closure Lifetime") {
198200
}
199201
c1.release()
200202
// Call a released closure
201-
_ = try expectThrow(try c1.throws())
203+
_ = try expectThrow(try evalClosure.throws(c1))
202204
}
203205

204206
do {
@@ -207,17 +209,17 @@ try test("Closure Lifetime") {
207209
_ = JSClosure { _ in .undefined }
208210
return .undefined
209211
}
210-
_ = try expectThrow(try c1.throws())
212+
_ = try expectThrow(try evalClosure.throws(c1))
211213
c1.release()
212214
}
213215

214216
do {
215217
let c1 = JSOneshotClosure { _ in
216218
return .boolean(true)
217219
}
218-
try expectEqual(c1(), .boolean(true))
220+
try expectEqual(evalClosure(c1), .boolean(true))
219221
// second call will cause `fatalError` that can be caught as a JavaScript exception
220-
_ = try expectThrow(try c1.throws())
222+
_ = try expectThrow(try evalClosure.throws(c1))
221223
// OneshotClosure won't call fatalError even if it's deallocated before `release`
222224
}
223225
}
@@ -244,7 +246,7 @@ try test("Host Function Registration") {
244246
return .number(1)
245247
}
246248

247-
setJSValue(this: prop_6Ref, name: "host_func_1", value: .function(hostFunc1))
249+
setJSValue(this: prop_6Ref, name: "host_func_1", value: .object(hostFunc1))
248250

249251
let call_host_1 = getJSValue(this: prop_6Ref, name: "call_host_1")
250252
let call_host_1Func = try expectFunction(call_host_1)
@@ -262,8 +264,8 @@ try test("Host Function Registration") {
262264
}
263265
}
264266

265-
try expectEqual(hostFunc2(3), .number(6))
266-
_ = try expectString(hostFunc2(true))
267+
try expectEqual(evalClosure(hostFunc2, 3), .number(6))
268+
_ = try expectString(evalClosure(hostFunc2, true))
267269
hostFunc2.release()
268270
}
269271

@@ -375,7 +377,7 @@ try test("ObjectRef Lifetime") {
375377

376378
let identity = JSClosure { $0[0] }
377379
let ref1 = getJSValue(this: .global, name: "globalObject1").object!
378-
let ref2 = identity(ref1).object!
380+
let ref2 = evalClosure(identity, ref1).object!
379381
try expectEqual(ref1.prop_2, .number(2))
380382
try expectEqual(ref2.prop_2, .number(2))
381383
identity.release()

IntegrationTests/bin/primary-tests.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ global.globalObject1 = {
4747
throw 3.0
4848
},
4949
},
50+
eval_closure: function(fn) {
51+
return fn(arguments[1])
52+
}
5053
};
5154

5255
global.Animal = function (name, age, isCat) {
Lines changed: 48 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,68 +1,68 @@
11
/** This timer is an abstraction over [`setInterval`](https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setInterval)
2-
/ [`clearInterval`](https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/clearInterval) and
2+
/ [`clearInterval`](https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/clearInterval) and
33
[`setTimeout`](https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setTimeout)
44
/ [`clearTimeout`](https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setTimeout)
55
JavaScript functions. It intentionally doesn't match the JavaScript API, as a special care is
66
needed to hold a reference to the timer closure and to call `JSClosure.release()` on it when the
77
timer is deallocated. As a user, you have to hold a reference to a `JSTimer` instance for it to stay
88
valid. The `JSTimer` API is also intentionally trivial, the timer is started right away, and the
99
only way to invalidate the timer is to bring the reference count of the `JSTimer` instance to zero.
10-
For invalidation you should either store the timer in an optional property and assign `nil` to it,
10+
For invalidation you should either store the timer in an optional property and assign `nil` to it,
1111
or deallocate the object that owns the timer.
1212
*/
1313
public final class JSTimer {
14-
/// Indicates whether this timer instance calls its callback repeatedly at a given delay.
15-
public let isRepeating: Bool
14+
/// Indicates whether this timer instance calls its callback repeatedly at a given delay.
15+
public let isRepeating: Bool
1616

17-
private let closure: JSOneshotClosure
17+
private let closure: JSClosureProtocol
1818

19-
/** Node.js and browser APIs are slightly different. `setTimeout`/`setInterval` return an object
20-
in Node.js, while browsers return a number. Fortunately, clearTimeout and clearInterval take
21-
corresponding types as their arguments, and we can store either as JSValue, so we can treat both
22-
cases uniformly.
23-
*/
24-
private let value: JSValue
25-
private let global = JSObject.global
19+
/** Node.js and browser APIs are slightly different. `setTimeout`/`setInterval` return an object
20+
in Node.js, while browsers return a number. Fortunately, clearTimeout and clearInterval take
21+
corresponding types as their arguments, and we can store either as JSValue, so we can treat both
22+
cases uniformly.
23+
*/
24+
private let value: JSValue
25+
private let global = JSObject.global
2626

27-
/**
28-
Creates a new timer instance that calls `setInterval` or `setTimeout` JavaScript functions for you
29-
under the hood.
30-
- Parameters:
31-
- millisecondsDelay: the amount of milliseconds before the `callback` closure is executed.
32-
- isRepeating: when `true` the `callback` closure is executed repeatedly at given
33-
`millisecondsDelay` intervals indefinitely until the timer is deallocated.
34-
- callback: the closure to be executed after a given `millisecondsDelay` interval.
35-
*/
36-
public init(millisecondsDelay: Double, isRepeating: Bool = false, callback: @escaping () -> ()) {
37-
if isRepeating {
38-
closure = JSClosure { _ in
39-
callback()
40-
return .undefined
27+
/**
28+
Creates a new timer instance that calls `setInterval` or `setTimeout` JavaScript functions for you
29+
under the hood.
30+
- Parameters:
31+
- millisecondsDelay: the amount of milliseconds before the `callback` closure is executed.
32+
- isRepeating: when `true` the `callback` closure is executed repeatedly at given
33+
`millisecondsDelay` intervals indefinitely until the timer is deallocated.
34+
- callback: the closure to be executed after a given `millisecondsDelay` interval.
35+
*/
36+
public init(millisecondsDelay: Double, isRepeating: Bool = false, callback: @escaping () -> ()) {
37+
if isRepeating {
38+
closure = JSClosure { _ in
39+
callback()
40+
return .undefined
41+
}
42+
} else {
43+
closure = JSOneshotClosure { _ in
44+
callback()
45+
return .undefined
46+
}
4147
}
42-
} else {
43-
closure = JSOneshotClosure { _ in
44-
callback()
45-
return .undefined
48+
self.isRepeating = isRepeating
49+
if isRepeating {
50+
value = global.setInterval.function!(closure, millisecondsDelay)
51+
} else {
52+
value = global.setTimeout.function!(closure, millisecondsDelay)
4653
}
4754
}
48-
self.isRepeating = isRepeating
49-
if isRepeating {
50-
value = global.setInterval.function!(closure, millisecondsDelay)
51-
} else {
52-
value = global.setTimeout.function!(closure, millisecondsDelay)
53-
}
54-
}
5555

56-
/** Makes a corresponding `clearTimeout` or `clearInterval` call, depending on whether this timer
57-
instance is repeating. The `closure` instance is released manually here, as it is required for
58-
bridged closure instances.
59-
*/
60-
deinit {
61-
if isRepeating {
62-
global.clearInterval.function!(value)
63-
} else {
64-
global.clearTimeout.function!(value)
56+
/** Makes a corresponding `clearTimeout` or `clearInterval` call, depending on whether this timer
57+
instance is repeating. The `closure` instance is released manually here, as it is required for
58+
bridged closure instances.
59+
*/
60+
deinit {
61+
if isRepeating {
62+
global.clearInterval.function!(value)
63+
} else {
64+
global.clearTimeout.function!(value)
65+
}
66+
closure.release()
6567
}
66-
closure.release()
67-
}
6868
}

Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift

Lines changed: 35 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,17 @@
11
import _CJavaScriptKit
22

3-
fileprivate var sharedFunctions: [JavaScriptHostFuncRef: ([JSValue]) -> JSValue] = [:]
3+
fileprivate var sharedClosures: [JavaScriptHostFuncRef: ([JSValue]) -> JSValue] = [:]
4+
5+
/// JSClosureProtocol abstracts closure object in JavaScript, whose lifetime is manualy managed
6+
public protocol JSClosureProtocol: JSValueCompatible {
7+
8+
/// Release this function resource.
9+
/// After calling `release`, calling this function from JavaScript will fail.
10+
func release()
11+
}
412

513
/// `JSOneshotClosure` is a JavaScript function that can be called only once.
6-
public class JSOneshotClosure: JSFunction {
14+
public class JSOneshotClosure: JSObject, JSClosureProtocol {
715
private var hostFuncRef: JavaScriptHostFuncRef = 0
816

917
public init(_ body: @escaping ([JSValue]) -> JSValue) {
@@ -12,7 +20,10 @@ public class JSOneshotClosure: JSFunction {
1220
let objectId = ObjectIdentifier(self)
1321
let funcRef = JavaScriptHostFuncRef(bitPattern: Int32(objectId.hashValue))
1422
// 2. Retain the given body in static storage by `funcRef`.
15-
sharedFunctions[funcRef] = body
23+
sharedClosures[funcRef] = {
24+
defer { self.release() }
25+
return body($0)
26+
}
1627
// 3. Create a new JavaScript function which calls the given Swift function.
1728
var objectRef: JavaScriptObjectRef = 0
1829
_create_function(funcRef, &objectRef)
@@ -21,15 +32,10 @@ public class JSOneshotClosure: JSFunction {
2132
id = objectRef
2233
}
2334

24-
public override func callAsFunction(this: JSObject? = nil, arguments: [ConvertibleToJSValue]) -> JSValue {
25-
defer { release() }
26-
return super.callAsFunction(this: this, arguments: arguments)
27-
}
28-
2935
/// Release this function resource.
3036
/// After calling `release`, calling this function from JavaScript will fail.
3137
public func release() {
32-
sharedFunctions[hostFuncRef] = nil
38+
sharedClosures[hostFuncRef] = nil
3339
}
3440
}
3541

@@ -52,30 +58,37 @@ public class JSOneshotClosure: JSFunction {
5258
/// eventListenter.release()
5359
/// ```
5460
///
55-
public class JSClosure: JSOneshotClosure {
56-
61+
public class JSClosure: JSObject, JSClosureProtocol {
62+
private var hostFuncRef: JavaScriptHostFuncRef = 0
5763
var isReleased: Bool = false
5864

5965
@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")
6066
@_disfavoredOverload
61-
public init(_ body: @escaping ([JSValue]) -> ()) {
62-
super.init({
67+
public convenience init(_ body: @escaping ([JSValue]) -> ()) {
68+
self.init({
6369
body($0)
6470
return .undefined
6571
})
6672
}
6773

68-
public override init(_ body: @escaping ([JSValue]) -> JSValue) {
69-
super.init(body)
70-
}
74+
public init(_ body: @escaping ([JSValue]) -> JSValue) {
75+
// 1. Fill `id` as zero at first to access `self` to get `ObjectIdentifier`.
76+
super.init(id: 0)
77+
let objectId = ObjectIdentifier(self)
78+
let funcRef = JavaScriptHostFuncRef(bitPattern: Int32(objectId.hashValue))
79+
// 2. Retain the given body in static storage by `funcRef`.
80+
sharedClosures[funcRef] = body
81+
// 3. Create a new JavaScript function which calls the given Swift function.
82+
var objectRef: JavaScriptObjectRef = 0
83+
_create_function(funcRef, &objectRef)
7184

72-
public override func callAsFunction(this: JSObject? = nil, arguments: [ConvertibleToJSValue]) -> JSValue {
73-
try! invokeJSFunction(self, arguments: arguments, this: this)
85+
hostFuncRef = funcRef
86+
id = objectRef
7487
}
75-
76-
public override func release() {
88+
89+
public func release() {
7790
isReleased = true
78-
super.release()
91+
sharedClosures[hostFuncRef] = nil
7992
}
8093

8194
deinit {
@@ -126,7 +139,7 @@ func _call_host_function_impl(
126139
_ argv: UnsafePointer<RawJSValue>, _ argc: Int32,
127140
_ callbackFuncRef: JavaScriptObjectRef
128141
) {
129-
guard let hostFunc = sharedFunctions[hostFuncRef] else {
142+
guard let hostFunc = sharedClosures[hostFuncRef] else {
130143
fatalError("The function was already released")
131144
}
132145
let arguments = UnsafeBufferPointer(start: argv, count: Int(argc)).map {

Sources/JavaScriptKit/JSValue.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ extension JSValue {
142142
/// ```
143143
@available(*, deprecated, message: "Please create JSClosure directly and manage its lifetime manually.")
144144
public static func function(_ body: @escaping ([JSValue]) -> JSValue) -> JSValue {
145-
.function(JSClosure(body))
145+
.object(JSClosure(body))
146146
}
147147
}
148148

0 commit comments

Comments
 (0)