W-20975340, W-21013345: SPM at checkout#3603
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
Ran linter on the files. Will be adding tests for all the changes. |
packages/template-retail-react-app/app/hooks/use-current-customer.js
Outdated
Show resolved
Hide resolved
packages/template-retail-react-app/app/pages/checkout/partials/sf-payments-sheet.jsx
Outdated
Show resolved
Hide resolved
packages/template-retail-react-app/app/pages/checkout/partials/sf-payments-sheet.jsx
Outdated
Show resolved
Hide resolved
packages/template-retail-react-app/app/pages/checkout/partials/sf-payments-sheet.jsx
Outdated
Show resolved
Hide resolved
packages/template-retail-react-app/app/pages/checkout/partials/sf-payments-sheet.jsx
Show resolved
Hide resolved
packages/template-retail-react-app/app/pages/checkout/partials/sf-payments-sheet.jsx
Outdated
Show resolved
Hide resolved
packages/template-retail-react-app/app/pages/checkout/partials/sf-payments-sheet.jsx
Outdated
Show resolved
Hide resolved
| * @param {Array<string>} [expand] - Optional array of fields to expand in the customer query | ||
| */ | ||
| export const useCurrentCustomer = () => { | ||
| export const useCurrentCustomer = (expand = undefined) => { |
|
|
||
| // Get the mocked useCustomer from commerce-sdk-react | ||
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| const mockUseCustomer = require('@salesforce/commerce-sdk-react').useCustomer |
There was a problem hiding this comment.
Is using require like this an established pattern in PWA?
There was a problem hiding this comment.
There are a few test modules (shipping-address.test.js, use-add-to-cart-modal.test.js) in PWA that use it.
There was a problem hiding this comment.
Feels suspect having to disable lint rule here. It'd be good to at least try to find another solution here in case this doesn't jive well with the PWA reviewers when we merge to develop. Otherwise PR is looking good and shaping up!
sf-mkosak
left a comment
There was a problem hiding this comment.
Overall LGTM. I would challenge you to try to remove the lint disabling in the test.
The lint disable is in place throughout the codebase wherever the require has been used. If not disabled, it throws a lint error: |
What I mean is change the implementation such that you don't use require and therefore don't need to override lint rules. |
Ah, got it, let me try that. Will update my upcoming PR with it. |
…/display-spms-at-checkout W-20975340, W-21013345: SPM at checkout
…/display-spms-at-checkout W-20975340, W-21013345: SPM at checkout
…/display-spms-at-checkout W-20975340, W-21013345: SPM at checkout
Display Saved Payment Methods at checkout
Includes refactoring for the work item W-21013345 as well.
Description
Currently supported SPMs are Credit cards and SEPA debit
Types of Changes
Changes
How to Test-Drive This PR
Checklists
General
Accessibility Compliance
You must check off all items in one of the follow two lists:
or...
Localization