Remove legacy checkout-bridge in swift#251
Conversation
React Native — Coverage Report
|
kiftio
changed the title
|
Approve of the fact we're removing the code relating to presented and instrumentation - was it necessary to move the rest of the CheckoutBridge into CheckoutWebview? Is this because android has moved away from using a checkoutbridge class? |
No! was a bit heavy handed, bringing it back |
kiftio
force-pushed
the
remove-obsolete-checkout-bridge
branch
from
3bb618e to
eca3130
Compare
kiftio
force-pushed
the
remove-obsolete-checkout-bridge
branch
2 times, most recently
from
8446081 to
64ec0c4
Compare
| func test_init_withNilEntryPoint_shouldSetCorrectUserAgent() { | ||
| let viewController = CheckoutWebViewController(checkoutURL: url, entryPoint: nil) | ||
|
|
||
| let expectedUserAgent = CheckoutBridge.applicationName(entryPoint: nil) |
There was a problem hiding this comment.
Not sure this line should change? The agent is set to CheckoutBridge.applicationName configuration.applicationNameForUserAgent = CheckoutBridge.applicationName(entryPoint: entryPoint)
| class CheckoutBridgeTests: XCTestCase { | ||
| func testReturnsStandardUserAgent() { | ||
| let version = ShopifyCheckoutKit.version | ||
| XCTAssertEqual(CheckoutBridge.applicationName, "ShopifyCheckoutKit/\(version) (iOS;Swift \(SwiftVersion.current!))") |
There was a problem hiding this comment.
applicationName wasn't deleted - should this test remain?
There was a problem hiding this comment.
We have these more focussed tests -> https://github.com/Shopify/checkout-kit/blob/main/platforms/swift/Tests/ShopifyCheckoutKitTests/UserAgentTests.swift
| window.console.error('EmbeddedCheckoutProtocol.postMessage is not available.'); | ||
| } | ||
| } catch (error) { | ||
| if (window && window.console && window.console.error) { | ||
| window.console.error('Failed to post message to checkout', error); |
There was a problem hiding this comment.
Let's add some prefixes to these logs to make it obvious that they came from Checkout Kit
| })(); | ||
| """ | ||
|
|
||
| webView.evaluateJavaScript(script) |
There was a problem hiding this comment.
If we're still dispatching the evaluateJavascript we are missing a test case here
Feels like we could still do with a sure test on this - sendMessage has the same issue as before in that it is a sync method with an async body - if we add a completion handler arg to sendResponse and forward that to webView.evaluateJavascript(script, completionHandler) we can write a test without the wait for flake
There was a problem hiding this comment.
Yeah, I was questioning whether we could live without testing the call to evaluateJavaScript, but we can add it if preferred
kiftio
force-pushed
the
remove-obsolete-checkout-bridge
branch
from
64ec0c4 to
f82d17a
Compare
3 participants
What changes are you making?
Remove the legacy Checkout Bridge, I don't believe we should be sending messages via
window.MobileCheckoutSdk.dispatchMessagein this version of the kit.If we intend to keep these messages, we'll need to incorporate them into the new protocol
How to test
n/a
Before you merge
Important
platforms/swift/README.mdand/orplatforms/android/README.md)Releasing a new Swift version?
ShopifyCheckoutKit.podspecplatforms/swift/Sources/ShopifyCheckoutKit/ShopifyCheckoutKit.swiftplatforms/swift/CHANGELOG.mdplatforms/swift/README.md(major version only)Releasing a new Android version?
versionNameinplatforms/android/lib/build.gradleplatforms/android/CHANGELOG.mdplatforms/android/README.mdTip
See the Contributing documentation for the full release process per platform.