[Sample] Add vaulted state settings to checkout as guest / static / customer accounts#428
[Sample] Add vaulted state settings to checkout as guest / static / customer accounts#428kieran-osgood-shopify merged 12 commits intomainfrom
Conversation
Coverage Report
|
kieran-osgood-shopify
force-pushed
the
feat/customer-auth
branch
from
002c2d7 to
2b27cb5
Compare
| }, [shopify, eventHandlers]); | ||
|
|
||
| return ( | ||
| <ConfigProvider |
There was a problem hiding this comment.
There was a bug introduced with a Provider refactor that meant we had providers incorrectly ordered - AppWithCheckoutKit would never have access to the Config because it was placed as a child
kieran-osgood-shopify
force-pushed
the
feat/customer-auth
branch
from
4a41206 to
9f1909b
Compare
| :ccache_enabled => true | ||
| ) | ||
|
|
||
| # WORKAROUND |
There was a problem hiding this comment.
Moving the pod inside the target removes the need for this workaround
|
|
||
| target 'ReactNativeTests' do | ||
| inherit! :complete | ||
| inherit! :search_paths |
There was a problem hiding this comment.
Newer modules (nitro modules) have checking to ensure there aren't duplicate dependencies linked, which :complete caused because its within the ReactNative target, just inheriting the search_paths makes sense here as he tests run on the host app
markmur
left a comment
markmur
left a comment
There was a problem hiding this comment.
Looks good, but I think it would help to better organise the functions using self-contained classes. Easier to lift and shift into a merchant app that way.
If we have time I think we should try squeeze this in before we ship but won't block on it.
| function getShopId(): string { | ||
| const shopId = Config.CUSTOMER_ACCOUNT_API_SHOP_ID; | ||
| if (!shopId) { | ||
| throw new Error('CUSTOMER_ACCOUNT_API_SHOP_ID is not configured'); | ||
| } | ||
| return shopId; | ||
| } | ||
|
|
||
| function getClientId(): string { | ||
| const clientId = Config.CUSTOMER_ACCOUNT_API_CLIENT_ID; | ||
| if (!clientId) { | ||
| throw new Error('CUSTOMER_ACCOUNT_API_CLIENT_ID is not configured'); | ||
| } | ||
| return clientId; | ||
| } | ||
|
|
||
| function getAuthorizationEndpoint(): string { | ||
| return `https://shopify.com/authentication/${getShopId()}/oauth/authorize`; | ||
| } | ||
|
|
||
| function getTokenEndpoint(): string { | ||
| return `https://shopify.com/authentication/${getShopId()}/oauth/token`; | ||
| } | ||
|
|
||
| function getLogoutEndpoint(): string { | ||
| return `https://shopify.com/authentication/${getShopId()}/logout`; | ||
| } | ||
|
|
||
| export function getRedirectUri(): string { | ||
| return `shop.${getShopId()}.app://callback`; | ||
| } | ||
|
|
||
| export function getCallbackScheme(): string { | ||
| return `shop.${getShopId()}.app`; | ||
| } |
There was a problem hiding this comment.
Could we use a class for all of the functions in this file? Some functions seem private while others are exported. It would be cleaner to separate the two using a class. Any of the computed URLs can come from getter methods
sample/src/auth/pkce.ts
Outdated
| export function generateCodeVerifier(): string { | ||
| const bytes = crypto.randomBytes(32); | ||
| return base64URLEncode(bytes.buffer); | ||
| } | ||
|
|
||
| export function generateCodeChallenge(verifier: string): string { | ||
| const hash = crypto.createHash('sha256').update(verifier).digest(); | ||
| return base64URLEncode(hash.buffer); | ||
| } | ||
|
|
||
| export function generateState(): string { | ||
| const bytes = crypto.randomBytes(27); | ||
| return base64URLEncode(bytes.buffer); | ||
| } |
There was a problem hiding this comment.
Might also make sense to group these in a class
sample/src/context/Auth.tsx
Outdated
| await customerAccountManager.logout(); | ||
| setSession(defaultSession); |
There was a problem hiding this comment.
Could the customerAccountManager manage the session? 🤔
There was a problem hiding this comment.
We use jotai in the sample already, so you could manage computed atoms in the class similarly to how we use signals
2 participants
What changes are you making?
Matches the implementation as in: https://github.com/Shopify/checkout-sheet-kit-swift/pull/523/changes
Adds setting to set the buyerIdentity for the cart, this allows testing of guest / hardCoded (.env values) / customerAccounts
Theres a number of changes to ensure that the sample appropriately logs out users when switching accounts, including a bug fix in the Context Providers where we never loaded dynamic values (used default context values) because of Context nesting order
This will allow testing for #427
Simulator.Screen.Recording.-.iPhone.16.Plus.-.2026-02-06.at.14.37.49.mov
PR Checklist
Important
Releasing a new version of the kit?
package.jsonfile.Tip
See the Contributing documentation for instructions on how to publish a new version of the library.