Skip to content
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

Async javascript take 2 #29

Merged
merged 29 commits into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
8af3967
Use a private encoder to compare messages, if needed
olivaresf Nov 17, 2023
e3c4ae8
Remove not necessary encoder and simplify equality function.
svara Dec 7, 2023
bdafa3f
Convert all JavaScript calls to async functions
joemasilotti Dec 7, 2023
8b4e7f7
evaluateJavaScript must be called from main thread
joemasilotti Dec 10, 2023
28dd478
Only import public API when testing public API
joemasilotti Dec 10, 2023
8c6872a
Ignore .swiftpm directory
joemasilotti Feb 26, 2024
5c7b8b5
Expose error entirely up stack
joemasilotti Feb 26, 2024
77ea025
Ignore xcuserdata
joemasilotti Feb 26, 2024
2a9aa02
Expose non-async functions to public API
joemasilotti Feb 26, 2024
52de2a3
Return non optional value from `evaluate(javaScript: String)`.
svara Feb 27, 2024
e9ce397
Remove obsolete completion handler.
svara Feb 27, 2024
36f9b17
Fix failing BridgeTests.
svara Feb 27, 2024
b2fbbfa
Update `BridgeDelegateSpy` to conform to `BridgingDelegate` protocol.
svara Feb 27, 2024
814213a
Increase expectations timeout interval to 5s.
svara Feb 27, 2024
9cee80f
Merge pull request #30 from hotwired/async-javascript-take-2-feedback
joemasilotti Feb 27, 2024
63ca1f0
Improve reliability of flaky tests
joemasilotti Feb 27, 2024
3b083b0
Completion blocks and no more flaky tests!
joemasilotti Feb 27, 2024
64b17d7
Add all missing non async versions of `reply(to:,with:)`.
svara Feb 28, 2024
86ea6f5
Add tests for non async version of `reply(to:, with:)`.`
svara Feb 28, 2024
cb3cdb3
Merge pull request #31 from hotwired/async-javascript-take-2-replies
joemasilotti Feb 28, 2024
390aad4
Make `BridgeComponent` run on main thread by adopting @MainActor attr…
svara Feb 28, 2024
5d58ad4
Make `BridgeDelegate` run on main thread by adopting @MainActor attri…
svara Feb 28, 2024
2364983
Make `Bridge` run on main thread by adopting @MainActor attribute. Fi…
svara Feb 28, 2024
894a8f1
Update tests to run on @MainActor.
svara Feb 28, 2024
e83e7b4
Don't dispatch script messages in an async context.
svara Feb 29, 2024
454d6c0
Move expectation timeout interval to its own file.
svara Feb 29, 2024
5d1cffb
Remove @MainActor attribute from the Bridge and add it to relevant fu…
svara Feb 29, 2024
40b3561
Merge branch 'async-javascript-take-2' into thread-safety
joemasilotti Mar 1, 2024
5ef73bc
Merge branch 'thread-safety' into async-javascript-take-2
joemasilotti Mar 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
build
node_modules
*.log

*.xcuserstate

*.xcbkptlist
.swiftpm
xcuserdata
145 changes: 83 additions & 62 deletions Source/Bridge.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,20 @@ public enum BridgeError: Error {
protocol Bridgable: AnyObject {
var delegate: BridgeDelegate? { get set }
var webView: WKWebView? { get }
func register(component: String)
func register(components: [String])
func unregister(component: String)
func reply(with message: Message)

func register(component: String) async throws
func register(components: [String]) async throws
func unregister(component: String) async throws
func reply(with message: Message) async throws
}

/// `Bridge` is the object for configuring a web view and
/// the channel for sending/receiving messages
public final class Bridge: Bridgable {
typealias CompletionHandler = (_ result: Any?, _ error: Error?) -> Void

public typealias InitializationCompletionHandler = () -> Void
weak var delegate: BridgeDelegate?
weak var webView: WKWebView?

public static func initialize(_ webView: WKWebView) {
if getBridgeFor(webView) == nil {
initialize(Bridge(webView: webView))
Expand All @@ -33,39 +32,39 @@ public final class Bridge: Bridgable {
self.webView = webView
loadIntoWebView()
}

deinit {
webView?.configuration.userContentController.removeScriptMessageHandler(forName: scriptHandlerName)
}


// MARK: - Internal API

/// Register a single component
/// - Parameter component: Name of a component to register support for
func register(component: String) {
callBridgeFunction(.register, arguments: [component])
@MainActor
func register(component: String) async throws {
try await callBridgeFunction(.register, arguments: [component])
}

/// Register multiple components
/// - Parameter components: Array of component names to register
func register(components: [String]) {
callBridgeFunction(.register, arguments: [components])
@MainActor
func register(components: [String]) async throws {
try await callBridgeFunction(.register, arguments: [components])
}

/// Unregister support for a single component
/// - Parameter component: Component name
func unregister(component: String) {
callBridgeFunction(.unregister, arguments: [component])
@MainActor
func unregister(component: String) async throws {
try await callBridgeFunction(.unregister, arguments: [component])
}

/// Send a message through the bridge to the web application
/// - Parameter message: Message to send
func reply(with message: Message) {
@MainActor
func reply(with message: Message) async throws {
logger.debug("bridgeWillReplyWithMessage: \(String(describing: message))")
let internalMessage = InternalMessage(from: message)
callBridgeFunction(.replyWith, arguments: [internalMessage.toJSON()])
try await callBridgeFunction(.replyWith, arguments: [internalMessage.toJSON()])
}

// /// Convenience method to reply to a previously received message. Data will be replaced,
// /// while id, component, and event will remain the same
// /// - Parameter message: Message to reply to
Expand All @@ -74,28 +73,27 @@ public final class Bridge: Bridgable {
// let replyMessage = message.replacing(data: data)
// callBridgeFunction("send", arguments: [replyMessage.toJSON()])
// }

/// Evaluates javaScript string directly as passed in sending through the web view
func evaluate(javaScript: String, completion: CompletionHandler? = nil) {
guard let webView = webView else {
completion?(nil, BridgeError.missingWebView)
return
@discardableResult
@MainActor
func evaluate(javaScript: String) async throws -> Any? {
joemasilotti marked this conversation as resolved.
Show resolved Hide resolved
guard let webView else {
throw BridgeError.missingWebView
}

webView.evaluateJavaScript(javaScript) { result, error in
if let error = error {
logger.error("Error evaluating JavaScript: \(error)")
}

completion?(result, error)

do {
return try await webView.evaluateJavaScriptAsync(javaScript)
} catch {
logger.error("Error evaluating JavaScript: \(error)")
throw error
}
}

/// Evaluates a JavaScript function with optional arguments by encoding the arguments
/// Function should not include the parens
/// Usage: evaluate(function: "console.log", arguments: ["test"])
func evaluate(function: String, arguments: [Any] = [], completion: CompletionHandler? = nil) {
evaluate(javaScript: JavaScript(functionName: function, arguments: arguments), completion: completion)
@MainActor
func evaluate(function: String, arguments: [Any] = []) async throws -> Any? {
try await evaluate(javaScript: JavaScript(functionName: function, arguments: arguments).toString())
}

static func initialize(_ bridge: Bridge) {
Expand All @@ -106,23 +104,24 @@ public final class Bridge: Bridgable {
static func getBridgeFor(_ webView: WKWebView) -> Bridge? {
return instances.first { $0.webView == webView }
}

// MARK: Private

private static var instances: [Bridge] = []
/// This needs to match whatever the JavaScript file uses
private let bridgeGlobal = "window.nativeBridge"

/// The webkit.messageHandlers name
private let scriptHandlerName = "strada"

private func callBridgeFunction(_ function: JavaScriptBridgeFunction, arguments: [Any]) {

@MainActor
private func callBridgeFunction(_ function: JavaScriptBridgeFunction, arguments: [Any]) async throws {
let js = JavaScript(object: bridgeGlobal, functionName: function.rawValue, arguments: arguments)
evaluate(javaScript: js)
try await evaluate(javaScript: js)
}

// MARK: - Configuration

/// Configure the bridge in the provided web view
private func loadIntoWebView() {
guard let configuration = webView?.configuration else { return }
Expand All @@ -131,17 +130,18 @@ public final class Bridge: Bridgable {
if let userScript = makeUserScript() {
configuration.userContentController.addUserScript(userScript)
}

let scriptMessageHandler = ScriptMessageHandler(delegate: self)
configuration.userContentController.add(scriptMessageHandler, name: scriptHandlerName)
}

private func makeUserScript() -> WKUserScript? {
guard
let path = PathLoader().pathFor(name: "strada", fileType: "js") else {
return nil
let path = PathLoader().pathFor(name: "strada", fileType: "js")
else {
return nil
}

do {
let source = try String(contentsOfFile: path)
return WKUserScript(source: source, injectionTime: .atDocumentStart, forMainFrameOnly: true)
Expand All @@ -150,18 +150,20 @@ public final class Bridge: Bridgable {
return nil
}
}

// MARK: - JavaScript Evaluation

private func evaluate(javaScript: JavaScript, completion: CompletionHandler? = nil) {

@discardableResult
@MainActor
private func evaluate(javaScript: JavaScript) async throws -> Any? {
do {
evaluate(javaScript: try javaScript.toString(), completion: completion)
return try await evaluate(javaScript: javaScript.toString())
} catch {
logger.error("Error evaluating JavaScript: \(String(describing: javaScript)), error: \(error)")
completion?(nil, error)
throw error
}
}

private enum JavaScriptBridgeFunction: String {
case register
case unregister
Expand All @@ -170,18 +172,37 @@ public final class Bridge: Bridgable {
}

extension Bridge: ScriptMessageHandlerDelegate {
@MainActor
func scriptMessageHandlerDidReceiveMessage(_ scriptMessage: WKScriptMessage) {
if let event = scriptMessage.body as? String,
event == "ready" {
if let event = scriptMessage.body as? String, event == "ready" {
delegate?.bridgeDidInitialize()
return
}

if let message = InternalMessage(scriptMessage: scriptMessage) {
delegate?.bridgeDidReceiveMessage(message.toMessage())
return
}

logger.warning("Unhandled message received: \(String(describing: scriptMessage.body))")
}
}

private extension WKWebView {
/// NOTE: The async version crashes the app with `Fatal error: Unexpectedly found nil while implicitly unwrapping an Optional value`
/// in case the function doesn't return anything.
/// This is a workaround. See https://forums.developer.apple.com/forums/thread/701553 for more details.
@discardableResult
@MainActor
func evaluateJavaScriptAsync(_ javaScriptString: String) async throws -> Any? {
return try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<Any?, Error>) in
evaluateJavaScript(javaScriptString) { data, error in
if let error {
continuation.resume(throwing: error)
} else {
continuation.resume(returning: data)
}
}
}
}
}
Loading
Loading