-
-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add JSTimer implementation with tests #46
Conversation
public final class JSTimer {
public init(_ after: TimeInterval, _ callback: @escaping () -> ())
public static repeating(_ interval: TimeInterval, _ callback: @escaping () -> ())
} |
It's reasonable. But unfortunately, |
I'm not sure this would work. public final class JSTimer {
public init(millisecondsDelay: Double, isRepeating: Bool = false, callback: @escaping () -> ())
public static func repeating(millisecondsDelay: Double, callback: @escaping () -> ())
} Maybe we could just add a default |
Depends on #45. This is also a prerequisite for a future
JSPromise
PR with tests.I intentionally didn't match the JS API in this PR, as a special care is needed to hold a reference to the timer closure and to call
.release()
on it. Here, a user is supposed to hold a reference to aJSTimer
instance for it to stay valid. The API is also intentionally simple, the timer is started right away, and the only way to invalidate the timer is to bring its reference count to zero.If you see a better way to manage closures passed to
setTimeout
andsetInterval
, I'd be happy to consider that.Also, Node.js and browser APIs are slightly different.
setTimeout
/setInterval
return an object in Node.js, while browsers return a number. Fortunately,clearTimeout
andclearInterval
take corresponding types as their arguments, and we can store either asJSValue
, so we can treat both cases uniformly.