Create mocks for the entire checkout extension API#3902
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
5c62f76 to
d914c44
Compare
247ecdf to
844aff7
Compare
844aff7 to
c8b1e1c
Compare
15dff41 to
b2a237f
Compare
1f8f47e to
562a606
Compare
b2a237f to
c9897ed
Compare
562a606 to
a54bddd
Compare
433f8fd to
aa7b07b
Compare
a54bddd to
07d5a50
Compare
aea8398 to
604bf1b
Compare
07d5a50 to
ffbd126
Compare
ae65d79 to
70c9a6c
Compare
d45cd12 to
d71f194
Compare
88fa431 to
b22ddd7
Compare
d71f194 to
8fcac33
Compare
315260c to
dd6200e
Compare
625b7aa to
040affd
Compare
6062317 to
e334238
Compare
040affd to
6fdecec
Compare
e334238 to
2ea50ce
Compare
58f1a20 to
8966550
Compare
2ea50ce to
a63e953
Compare
a77c904 to
17937cc
Compare
a63e953 to
40027b7
Compare
17937cc to
f2af9e8
Compare
b51dc56 to
e98803a
Compare
883ccce to
8751012
Compare
e98803a to
bdf2921
Compare
fc22751 to
5830102
Compare
bdf2921 to
2388be3
Compare
jamesvidler
left a comment
There was a problem hiding this comment.
This is really great work! I'd like to see us add some type safety for the targets so we don't get into scenarios where we forget to update the mocks as targets change.
| @@ -0,0 +1,29 @@ | |||
| { | |||
| "name": "checkout-basic-testing-example", | |||
| "version": "1.0.0", | |||
There was a problem hiding this comment.
I had to add
"type": "module",
Otherwise I got:
Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/jkv/src/github.com/Shopify/ui-extensions/examples/testing/checkout-basic-testing-example/node_modules/vite/dist/node/index.js from /Users/jkv/src/github.com/Shopify/ui-extensions/examples/testing/checkout-basic-testing-example/node_modules/vitest/dist/config.cjs not supported.
Instead change the require of index.js in /Users/jkv/src/github.com/Shopify/ui-extensions/examples/testing/checkout-basic-testing-example/node_modules/vitest/dist/config.cjs to a dynamic import() which is available in all CommonJS modules.
at _require.extensions.<computed> [as .js] (file:///Users/jkv/src/github.com/Shopify/ui-extensions/examples/testing/checkout-basic-testing-example/node_modules/vite/dist/node/chunks/config.js:35920:9)
at Object.<anonymous> (/Users/jkv/src/github.com/Shopify/ui-extensions/examples/testing/checkout-basic-testing-example/node_modules/vitest/dist/config.cjs:5:12)
at _require.extensions.<computed> [as .js] (file:///Users/jkv/src/github.com/Shopify/ui-extensions/examples/testing/checkout-basic-testing-example/node_modules/vite/dist/node/chunks/config.js:35920:9) {
code: 'ERR_REQUIRE_ESM'
}
When running the tests in here.
| const checkoutMockFactories: Partial<CheckoutMockFactory> = { | ||
| // CheckoutApi & StandardApi | ||
| 'purchase.checkout.actions.render-before': createCheckoutStandardMock, | ||
| 'Checkout::Actions::RenderBefore': createCheckoutStandardMock, |
There was a problem hiding this comment.
Do you think we need to support the legacy targets here? I thought we removed these types from the library in latest versions?
There was a problem hiding this comment.
Ah, these snuck in. The types weren't removed from the library because it was too hard to support removal in checkout-web when I tried. Maybe we should ask River to try it again :thinking_face:
There was a problem hiding this comment.
I'll need to leave them in for now since the typing is exhaustive.
| extension.shopify.lines.value = [ | ||
| createCartLine({quantity: 2}), | ||
| createCartLine({ | ||
| id: 'gid://shopify/CartLine/2', | ||
| merchandise: { | ||
| type: 'variant', | ||
| id: 'gid://shopify/ProductVariant/99', | ||
| title: 'Large Widget', | ||
| selectedOptions: [], | ||
| product: { | ||
| id: 'gid://shopify/Product/42', | ||
| vendor: 'Acme', | ||
| productType: 'widget', | ||
| }, | ||
| requiresShipping: true, | ||
| }, | ||
| }), | ||
| ]; |
There was a problem hiding this comment.
Would this also set the .current internally that we expose for backwards compatibility? Or, do we still expose that - I haven't used remote-dom too much lately.
There was a problem hiding this comment.
No we don't document that. We should have just removed it entirely. That was probably a mistake. We don't need to support it in the mocks.

Follow up to #3899 that adds a complete, type-safe mock of the checkout extension API
👁️ How to review this PR