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 17 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
122 changes: 60 additions & 62 deletions Source/Bridge.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,64 +8,62 @@ 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

weak var delegate: BridgeDelegate?
weak var webView: WKWebView?

public static func initialize(_ webView: WKWebView) {
if getBridgeFor(webView) == nil {
initialize(Bridge(webView: webView))
}
}

init(webView: WKWebView) {
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])
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])
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])
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) {
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 +72,26 @@ 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
@MainActor
@discardableResult
func evaluate(javaScript: String) async throws -> Any {
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.evaluateJavaScript(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)
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 +102,23 @@ 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]) {

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 +127,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 +147,19 @@ public final class Bridge: Bridgable {
return nil
}
}

// MARK: - JavaScript Evaluation

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

@discardableResult
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 +168,18 @@ public final class Bridge: Bridgable {
}

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

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

logger.warning("Unhandled message received: \(String(describing: scriptMessage.body))")
}
}
54 changes: 42 additions & 12 deletions Source/BridgeComponent.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ protocol BridgingComponent: AnyObject {
}

open class BridgeComponent: BridgingComponent {
public typealias ReplyCompletionHandler = (Result<Bool, Error>) -> Void
joemasilotti marked this conversation as resolved.
Show resolved Hide resolved

/// A unique name representing the `BridgeComponent` type.
///
/// Subclasses must provide their own implementation of this property.
Expand Down Expand Up @@ -50,26 +52,56 @@ open class BridgeComponent: BridgingComponent {
///
/// - Parameter message: The message to be replied with.
/// - Returns: `true` if the reply was successful, `false` if the bridge is not available.
public func reply(with message: Message) -> Bool {
return delegate.reply(with: message)
public func reply(with message: Message) async throws -> Bool {
try await delegate.reply(with: message)
}


/// Replies to the web with a received message, optionally replacing its `event` or `jsonData`.
///
/// - Parameter message: The message to be replied with.
public func reply(with message: Message, completion: ReplyCompletionHandler? = nil) {
joemasilotti marked this conversation as resolved.
Show resolved Hide resolved
Task {
do {
let result = try await delegate.reply(with: message)
completion?(.success((result)))
} catch {
completion?(.failure(error))
}
}
}

@discardableResult
/// Replies to the web with the last received message for a given `event` with its original `jsonData`.
///
/// NOTE: If a message has not been received for the given `event`, the reply will be ignored.
///
/// - Parameter event: The `event` for which a reply should be sent.
/// - Returns: `true` if the reply was successful, `false` if the event message was not received.
public func reply(to event: String) -> Bool {
public func reply(to event: String) async throws -> Bool {
guard let message = receivedMessage(for: event) else {
logger.warning("bridgeMessageFailedToReply: message for event \(event) was not received")
return false
}

return reply(with: message)
return try await reply(with: message)
}


/// Replies to the web with the last received message for a given `event` with its original `jsonData`.
///
/// NOTE: If a message has not been received for the given `event`, the reply will be ignored.
///
/// - Parameter event: The `event` for which a reply should be sent.
public func reply(to event: String, completion: ReplyCompletionHandler? = nil) {
Task {
do {
let result = try await reply(to: event)
completion?(.success((result)))
} catch {
completion?(.failure(error))
}
}
}

@discardableResult
/// Replies to the web with the last received message for a given `event`, replacing its `jsonData`.
///
Expand All @@ -79,15 +111,14 @@ open class BridgeComponent: BridgingComponent {
/// - event: The `event` for which a reply should be sent.
/// - jsonData: The `jsonData` to be included in the reply message.
/// - Returns: `true` if the reply was successful, `false` if the event message was not received.
public func reply(to event: String, with jsonData: String) -> Bool {
public func reply(to event: String, with jsonData: String) async throws -> Bool {
guard let message = receivedMessage(for: event) else {
logger.warning("bridgeMessageFailedToReply: message for event \(event) was not received")
return false
}

let messageReply = message.replacing(jsonData: jsonData)

return reply(with: messageReply)
return try await reply(with: messageReply)
}

@discardableResult
Expand All @@ -100,15 +131,14 @@ open class BridgeComponent: BridgingComponent {
/// - event: The `event` for which a reply should be sent.
/// - data: An instance conforming to `Encodable` to be included as `jsonData` in the reply message.
/// - Returns: `true` if the reply was successful, `false` if the event message was not received.
public func reply<T: Encodable>(to event: String, with data: T) -> Bool {
public func reply<T: Encodable>(to event: String, with data: T) async throws -> Bool {
guard let message = receivedMessage(for: event) else {
logger.warning("bridgeMessageFailedToReply: message for event \(event) was not received")
return false
}

let messageReply = message.replacing(data: data)

return reply(with: messageReply)
return try await reply(with: messageReply)
}

/// Returns the last received message for a given `event`, if available.
Expand Down
14 changes: 7 additions & 7 deletions Source/BridgeDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ public protocol BridgingDelegate: AnyObject {

func webViewDidBecomeActive(_ webView: WKWebView)
func webViewDidBecomeDeactivated()
func reply(with message: Message) -> Bool
func reply(with message: Message) async throws -> Bool

func onViewDidLoad()
func onViewWillAppear()
func onViewDidAppear()
Expand All @@ -20,7 +20,7 @@ public protocol BridgingDelegate: AnyObject {

func component<C: BridgeComponent>() -> C?

func bridgeDidInitialize()
func bridgeDidInitialize() async throws
func bridgeDidReceiveMessage(_ message: Message) -> Bool
}

Expand Down Expand Up @@ -60,13 +60,13 @@ public final class BridgeDelegate: BridgingDelegate {
///
/// - Parameter message: The message to be replied with.
/// - Returns: `true` if the reply was successful, `false` if the bridge is not available.
public func reply(with message: Message) -> Bool {
public func reply(with message: Message) async throws -> Bool {
guard let bridge else {
logger.warning("bridgeMessageFailedToReply: bridge is not available")
return false
}

bridge.reply(with: message)
try await bridge.reply(with: message)
return true
}

Expand Down Expand Up @@ -109,9 +109,9 @@ public final class BridgeDelegate: BridgingDelegate {

// MARK: Internal use

public func bridgeDidInitialize() {
public func bridgeDidInitialize() async throws {
let componentNames = componentTypes.map { $0.name }
bridge?.register(components: componentNames)
try await bridge?.register(components: componentNames)
}

@discardableResult
Expand Down
Loading
Loading